Re: [PATCH v1 net] net: macb: Use rcu_dereference() for idev->ifa_list in macb_suspend().

[Vineeth Karumanchi <vineeth.karumanchi@amd.com>]

Hi Kuniyuki,

On 08/08/24 9:30 am, Kuniyuki Iwashima wrote:
> In macb_suspend(), idev->ifa_list is fetched with rcu_access_pointer()
> and later the pointer is dereferenced as ifa->ifa_local.
> 
> So, idev->ifa_list must be fetched with rcu_dereference().
> 

Is there any functional breakage ?

I sent initial patch with rcu_dereference, but there is a review comment:

https://lore.kernel.org/netdev/a02fac3b21a97dc766d65c4ed2d080f1ed87e87e.camel@redhat.com/ 



🙏 vineeth

> Fixes: 0cb8de39a776 ("net: macb: Add ARP support to WOL")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
>   drivers/net/ethernet/cadence/macb_main.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 11665be3a22c..dcd3f54ed0cf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -5250,8 +5250,8 @@ static int __maybe_unused macb_suspend(struct device *dev)
>   	if (bp->wol & MACB_WOL_ENABLED) {
>   		/* Check for IP address in WOL ARP mode */
>   		idev = __in_dev_get_rcu(bp->dev);
> -		if (idev && idev->ifa_list)
> -			ifa = rcu_access_pointer(idev->ifa_list);
> +		if (idev)
> +			ifa = rcu_dereference(idev->ifa_list);
>   		if ((bp->wolopts & WAKE_ARP) && !ifa) {
>   			netdev_err(netdev, "IP address not assigned as required by WoL walk ARP\n");
>   			return -EOPNOTSUPP;