lkml.org 
[lkml]   [2017]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] netfilter: ip_tables: remove useless variable assignment in get_info()
Hi Joe,

Quoting Joe Perches <joe@perches.com>:

> On Mon, 2017-06-26 at 17:34 -0500, Gustavo A. R. Silva wrote:
>> Value assigned to variable _ret_ at line 970 is overwritten either at
>> line 986 or 988, before it can be used. This makes such variable
>> assignment useless.
>>
>> Addresses-Coverity-ID: 1226932
> []
>> diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
> []
>> @@ -967,7 +967,7 @@ static int get_info(struct net *net, void __user *user,
>> struct xt_table_info tmp;
>>
>> if (compat) {
>> - ret = compat_table_info(private, &tmp);
>> + compat_table_info(private, &tmp);
>
> why isn't it more appropriate to test the return value?
>

Oh, in this particular case, based on git blame, the code has been
like that for more than 10 years. So my reasoning was that if it
hasn't been fixed yet, maybe that return value is not relevant.

But in case it turns out to actually be relevant, what do you think
about the following patch:

--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -968,7 +968,8 @@ static int get_info(struct net *net, void __user *user,

if (compat) {
ret = compat_table_info(private, &tmp);
- xt_compat_flush_offsets(AF_INET);
+ if (!ret)
+ goto out;
private = &tmp;
}
#endif
@@ -986,14 +987,20 @@ static int get_info(struct net *net, void __user *user,
ret = -EFAULT;
else
ret = 0;
+ } else
+ ret = -ENOENT;

+out:
+ if (t) {
xt_table_unlock(t);
module_put(t->me);
- } else
- ret = -ENOENT;
+ }
+
#ifdef CONFIG_COMPAT
- if (compat)
+ if (compat) {
+ xt_compat_flush_offsets(AF_INET);
xt_compat_unlock(AF_INET);
+ }
#endif
return ret;
}

Thank you!
--
Gustavo A. R. Silva





\
 
 \ /
  Last update: 2017-06-27 05:42    [W:0.104 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site