mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: "'Arnd Bergmann'" <arnd@arndb.de>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>,
	Florian Westphal <fw@strlen.de>,
	"David S. Miller" <davem@davemloft.net>
Cc: Johannes Berg <johannes.berg@intel.com>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Aaron Conole <aconole@bytheb.org>,
	"netfilter-devel@vger.kernel.org"
	<netfilter-devel@vger.kernel.org>,
	"coreteam@netfilter.org" <coreteam@netfilter.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] netfilter: fix stringop-overflow warning with UBSAN
Date: Tue, 1 Aug 2017 09:25:13 +0000	[thread overview]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6DD0047F28@AcuExch.aculab.com> (raw)
In-Reply-To: <20170731100913.465530-1-arnd@arndb.de>

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 <arnd@arndb.de>
> ---
>  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

  reply	other threads:[~2017-08-01  9:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-31 10:09 Arnd Bergmann
2017-08-01  9:25 ` David Laight [this message]
2017-10-04 18:01   ` Jozsef Kadlecsik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=063D6719AE5E284EB5DD2968C1650D6DD0047F28@AcuExch.aculab.com \
    --to=david.laight@aculab.com \
    --cc=aconole@bytheb.org \
    --cc=adobriyan@gmail.com \
    --cc=arnd@arndb.de \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=fw@strlen.de \
    --cc=johannes.berg@intel.com \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®