Re: [PATCH nf-next,v2 3/5] net: do not advance stack index from dev_fwd_path()

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2962 bytes --]

> Update stack index from dev_fill_forward_path() instead, once the
> forward path slot has been populated.
> 
> Adjust existing drivers that recycle dev_fill_forward_path() as a
> way to reach the mtk SoC WDMA info.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
> v2: new in this series to address sashiko report on possible access
>     to uninitialized path in stack in case tunnel route reference
>     need to be dropped.
> 
>  drivers/net/ethernet/airoha/airoha_ppe.c        | 2 +-
>  drivers/net/ethernet/mediatek/mtk_ppe_offload.c | 2 +-
>  net/core/dev.c                                  | 6 ++++--
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
> index e4c3644dd6ec..72755ca974fa 100644
> --- a/drivers/net/ethernet/airoha/airoha_ppe.c
> +++ b/drivers/net/ethernet/airoha/airoha_ppe.c
> @@ -292,7 +292,7 @@ static int airoha_ppe_get_wdma_info(struct net_device *dev, const u8 *addr,
>  	if (err)
>  		return err;
>  
> -	path = &stack.path[stack.num_paths - 1];
> +	path = &stack.path[stack.num_paths];
>  	if (path->type != DEV_PATH_MTK_WDMA)
>  		return -EINVAL;
>  
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> index cc8c4ef8038f..7dab360b8be3 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> @@ -107,7 +107,7 @@ mtk_flow_get_wdma_info(struct net_device *dev, const u8 *addr, struct mtk_wdma_i
>  	if (err)
>  		return err;
>  
> -	path = &stack.path[stack.num_paths - 1];
> +	path = &stack.path[stack.num_paths];
>  	if (path->type != DEV_PATH_MTK_WDMA)
>  		return -1;
>  
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c1c1be1a6962..ca65d1dcd604 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -742,9 +742,9 @@ EXPORT_SYMBOL_GPL(dev_fill_metadata_dst);
>  
>  static struct net_device_path *dev_fwd_path(struct net_device_path_stack *stack)
>  {
> -	int k = stack->num_paths++;
> +	int k = stack->num_paths;
>  
> -	if (k >= NET_DEVICE_PATH_STACK_MAX)
> +	if (k + 1 >= NET_DEVICE_PATH_STACK_MAX)

nit: do we really need k in this case? (ignore the comment if you do not
need to repost).

Regards,
Lorenzo

>  		return NULL;
>  
>  	return &stack->path[k];
> @@ -773,6 +773,7 @@ int dev_fill_forward_path(const struct net_device *dev, const u8 *daddr,
>  		if (ret < 0)
>  			return -1;
>  
> +		stack->num_paths++;
>  		if (WARN_ON_ONCE(last_dev == ctx.dev))
>  			return -1;
>  	}
> @@ -785,6 +786,7 @@ int dev_fill_forward_path(const struct net_device *dev, const u8 *daddr,
>  		return -1;
>  	path->type = DEV_PATH_ETHERNET;
>  	path->dev = ctx.dev;
> +	stack->num_paths++;
>  
>  	return ret;
>  }
> -- 
> 2.47.3
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]