| Message ID | 1542838273-13703-1-git-send-email-cai@gmx.us |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: ipset: replace a strncpy() with strscpy() | expand |
Hi, On Wed, 21 Nov 2018, Qian Cai wrote: > To make overflows as obvious as possible and to prevent code from blithely > proceeding with a truncated string. This also has a side-effect to fix a > compilation warning using GCC 8.2.1. > > net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get': > net/netfilter/ipset/ip_set_core.c:2027:3: warning: 'strncpy' writing 32 > bytes into a region of size 2 overflows the destination > [-Wstringop-overflow=] But without checking the return value of strscpy(), I don't see why it's better to call strscpy(). Also, with your patch I get the warning: CC [M] /usr/src/git/ipset/ipset/kernel/net/netfilter/xt_set.o CC [M] /usr/src/git/ipset/ipset/kernel/net/netfilter/ipset/ip_set_core.o /usr/src/git/ipset/ipset/kernel/net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get': /usr/src/git/ipset/ipset/kernel/net/netfilter/ipset/ip_set_core.c:2184:3: warning: ignoring return value of 'strscpy', declared with attribute warn_unused_result [-Wunused-result] strscpy(req_get->set.name, set ? set->name : "", ^ So please add the proper checking of the return value. Best regards, Jozsef > Signed-off-by: Qian Cai <cai@gmx.us> > --- > net/netfilter/ipset/ip_set_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c > index 1577f2f..915aa0d 100644 > --- a/net/netfilter/ipset/ip_set_core.c > +++ b/net/netfilter/ipset/ip_set_core.c > @@ -2024,7 +2024,7 @@ static int ip_set_protocol(struct net *net, struct sock *ctnl, > } > nfnl_lock(NFNL_SUBSYS_IPSET); > set = ip_set(inst, req_get->set.index); > - strncpy(req_get->set.name, set ? set->name : "", > + strscpy(req_get->set.name, set ? set->name : "", > IPSET_MAXNAMELEN); > nfnl_unlock(NFNL_SUBSYS_IPSET); > goto copy; > -- > 1.8.3.1 > > - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c index 1577f2f..915aa0d 100644 --- a/net/netfilter/ipset/ip_set_core.c +++ b/net/netfilter/ipset/ip_set_core.c @@ -2024,7 +2024,7 @@ static int ip_set_protocol(struct net *net, struct sock *ctnl, } nfnl_lock(NFNL_SUBSYS_IPSET); set = ip_set(inst, req_get->set.index); - strncpy(req_get->set.name, set ? set->name : "", + strscpy(req_get->set.name, set ? set->name : "", IPSET_MAXNAMELEN); nfnl_unlock(NFNL_SUBSYS_IPSET); goto copy;
To make overflows as obvious as possible and to prevent code from blithely proceeding with a truncated string. This also has a side-effect to fix a compilation warning using GCC 8.2.1. net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get': net/netfilter/ipset/ip_set_core.c:2027:3: warning: 'strncpy' writing 32 bytes into a region of size 2 overflows the destination [-Wstringop-overflow=] Signed-off-by: Qian Cai <cai@gmx.us> --- net/netfilter/ipset/ip_set_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)