linux-kernel - Re: [PATCH] dsa: bcm_sf2_cfp: fix an incorrect NULL check on list iterator

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:   Sun, 27 Mar 2022 21:58:05 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Xiaomeng Tong <xiam0nd.tong@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>
Cc:     andrew@...n.ch, vivien.didelot@...il.com, davem@...emloft.net,
        kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] dsa: bcm_sf2_cfp: fix an incorrect NULL check on list
 iterator

On Sun, Mar 27, 2022 at 01:55:47PM +0800, Xiaomeng Tong wrote:
> The bug is here:
> 	return rule;
> 
> The list iterator value 'rule' will *always* be set and non-NULL
> by list_for_each_entry(), so it is incorrect to assume that the
> iterator value will be NULL if the list is empty or no element
> is found.
> 
> To fix the bug, return 'rule' when found, otherwise return NULL.
> 
> Cc: stable@...r.kernel.org
> Fixes: ae7a5aff783c7 ("net: dsa: bcm_sf2: Keep copy of inserted rules")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@...il.com>
> ---

The change looks correct, but from a process standpoint for next time
(a) you should have copied Florian, the driver's maintainer (which I did now)
    who appears on the top of the list in the output of ./get_maintainer.pl
(b) networking bugfixes that apply to the "net" tree shouldn't need
    stable@...r.kernel.org copied, instead just target the patch against
    the https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
    tree and mark the subject prefix as "[PATCH net]".

Reviewed-by: Vladimir Oltean <olteanv@...il.com>

>  drivers/net/dsa/bcm_sf2_cfp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
> index a7e2fcf2df2c..edbe5e7f1cb6 100644
> --- a/drivers/net/dsa/bcm_sf2_cfp.c
> +++ b/drivers/net/dsa/bcm_sf2_cfp.c
> @@ -567,14 +567,14 @@ static void bcm_sf2_cfp_slice_ipv6(struct bcm_sf2_priv *priv,
>  static struct cfp_rule *bcm_sf2_cfp_rule_find(struct bcm_sf2_priv *priv,
>  					      int port, u32 location)
>  {
> -	struct cfp_rule *rule = NULL;
> +	struct cfp_rule *rule;
>  
>  	list_for_each_entry(rule, &priv->cfp.rules_list, next) {
>  		if (rule->port == port && rule->fs.location == location)
> -			break;
> +			return rule;
>  	}
>  
> -	return rule;
> +	return NULL;
>  }
>  
>  static int bcm_sf2_cfp_rule_cmp(struct bcm_sf2_priv *priv, int port,
> -- 
> 2.17.1
>