linux-kernel - Re: linux-next: build warning after merge of the net tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 05 Dec 2019 14:57:39 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     sfr@...b.auug.org.au
Cc:     netdev@...r.kernel.org, linux-next@...r.kernel.org,
        linux-kernel@...r.kernel.org, sd@...asysnail.net
Subject: Re: linux-next: build warning after merge of the net tree

From: Stephen Rothwell <sfr@...b.auug.org.au>
Date: Thu, 5 Dec 2019 08:44:40 +1100

> After merging the net tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c: In function 'mlx5e_tc_tun_create_header_ipv6':
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c:332:20: warning: 'n' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   332 |  struct neighbour *n;
>       |                    ^
> 
> Introduced by commit
> 
>   6c8991f41546 ("net: ipv6_stub: use ip6_dst_lookup_flow instead of ip6_dst_lookup")
> 
> It looks like a false positive.

I think it is a false positive as well.  It looks like the compiler
has trouble seeing through ptr error guards.  Actually, the top level
logic looks generically like this:

Old code:

int foo(int *byref)
{
	int err, val;

	err = something(&val);
	if (err < 0)
		return err;

	*byref = val;
	return 0;
}

...
	struct whatever *obj;

	err = foo(&obj);
	if (err < 0)
		return err;
	a = obj->something;

New code:

int foo(int *byref)
{
	struct otherthing *x;

	x = something(&val);
	if (IS_ERR(x))
		return PTR_ERR(x);
	*byref = bar(x);
	return 0;
}

...
	struct whatever *obj;

	err = foo(&obj);
	if (err < 0)
		return err;
	a = obj->somethng;

In the new code the compiler can only see that the return value in the
error case is non-zero, not necessarily that it is < 0 which is the
guard against uninitialized accesses of 'obj'.

It will satisfy this property, but through the various casts and
implicit demotions, this information is lost on the compiler.

My compiler didn't emit this warning FWIW.

gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)

I'm unsure how to handle this, setting 'n' to explicitly be NULL
is bogus because the compiler now will think that a NULL deref
happens since the guard isn't guarding the existing assignment.