From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751912AbdHAJZV convert rfc822-to-8bit (ORCPT ); Tue, 1 Aug 2017 05:25:21 -0400 Received: from smtp-out6.electric.net ([192.162.217.182]:56850 "EHLO smtp-out6.electric.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750832AbdHAJZT (ORCPT ); Tue, 1 Aug 2017 05:25:19 -0400 From: David Laight To: "'Arnd Bergmann'" , Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , "David S. Miller" CC: Johannes Berg , Alexey Dobriyan , Aaron Conole , "netfilter-devel@vger.kernel.org" , "coreteam@netfilter.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH] netfilter: fix stringop-overflow warning with UBSAN Thread-Topic: [PATCH] netfilter: fix stringop-overflow warning with UBSAN Thread-Index: AQHTCeU0ELIJbxglt028HE10T/iu36JvO7yQ Date: Tue, 1 Aug 2017 09:25:13 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6DD0047F28@AcuExch.aculab.com> References: <20170731100913.465530-1-arnd@arndb.de> In-Reply-To: <20170731100913.465530-1-arnd@arndb.de> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.202.99.200] Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Outbound-IP: 156.67.243.126 X-Env-From: David.Laight@ACULAB.COM X-Proto: esmtps X-Revdns: X-HELO: AcuExch.aculab.com X-TLS: TLSv1:AES128-SHA:128 X-Authenticated_ID: X-PolicySMART: 3396946, 3397078 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Arnd Bergmann > Sent: 31 July 2017 11:09 > Using gcc-7 with UBSAN enabled, we get this false-positive warning: > > net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get': > net/netfilter/ipset/ip_set_core.c:1998:3: error: 'strncpy' writing 32 bytes into a region of size 2 > overflows the destination [-Werror=stringop-overflow=] > strncpy(req_get->set.name, set ? set->name : "", > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > sizeof(req_get->set.name)); > ~~~~~~~~~~~~~~~~~~~~~~~~~~ > > This seems completely bogus, and I could not find a nice workaround. > To work around it in a less elegant way, I change the ?: operator > into an if()/else() construct. > > Signed-off-by: Arnd Bergmann > --- > net/netfilter/ipset/ip_set_core.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c > index e495b5e484b1..d7ebb021003b 100644 > --- a/net/netfilter/ipset/ip_set_core.c > +++ b/net/netfilter/ipset/ip_set_core.c > @@ -1995,8 +1995,12 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len) > } > nfnl_lock(NFNL_SUBSYS_IPSET); > set = ip_set(inst, req_get->set.index); > - strncpy(req_get->set.name, , > - IPSET_MAXNAMELEN); > + if (set) > + strncpy(req_get->set.name, set->name, > + sizeof(req_get->set.name)); > + else > + memset(req_get->set.name, '\0', > + sizeof(req_get->set.name)); If you use strncpy() here, the compiler might optimise the code back to 'how it was before'. Or, maybe an explicit temporary: 'const char *name = set ? set->name : ""; David