netdev - Re: [PATCH net] net: dsa: don't dereference NULL extack in dsa_slave

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 22 Aug 2022 18:25:23 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Clément Léger <clement.leger@...tlin.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Sergei Antonov <saproj@...il.com>
Subject: Re: [PATCH net] net: dsa: don't dereference NULL extack in
 dsa_slave_changeupper()

On Fri, 19 Aug 2022 20:39:25 +0300 Vladimir Oltean wrote:
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index c548b969b083..804a00324c8b 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -2487,7 +2487,7 @@ static int dsa_slave_changeupper(struct net_device *dev,
>  			if (!err)
>  				dsa_bridge_mtu_normalization(dp);
>  			if (err == -EOPNOTSUPP) {
> -				if (!extack->_msg)
> +				if (extack && !extack->_msg)
>  					NL_SET_ERR_MSG_MOD(extack,
>  							   "Offloading not supported");

Other offload paths set the extack prior to the driver call,
which has the same effect. Can't we do the same thing here?
Do we care about preserving the extack from another notifier 
handler or something? Does not seem like that's the case judging 
but the commit under Fixes.

If it is the case (and hopefully not) we should add a new macro wrapper.
Manually twiddling with a field starting with an underscore makes
me feel dirty. Perhaps I have been writing too much python lately.