Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 27 Mar 2022 21:58:05 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Xiaomeng Tong <xiam0nd.tong@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>
Cc:     andrew@lunn.ch, vivien.didelot@gmail.com, davem@davemloft.net,
        kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] dsa: bcm_sf2_cfp: fix an incorrect NULL check on list
 iterator
Message-ID: <20220327185805.cibcmk4rejgb7jre@skbuf>
References: <20220327055547.3938-1-xiam0nd.tong@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220327055547.3938-1-xiam0nd.tong@gmail.com>
Precedence: bulk

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@vger.kernel.org
> Fixes: ae7a5aff783c7 ("net: dsa: bcm_sf2: Keep copy of inserted rules")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.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@vger.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@gmail.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
>