Re: [net,v2] ipv6: sr: Add NULL check for idev in seg6_hmac_validate_skb

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ipv6: sr: Add NULL check for idev in seg6_hmac_validate_skb

In seg6_hmac_validate_skb(), the pointer returned by __in6_dev_get()
can be NULL if the network device lacks proper IPv6 initialization
or is being torn down. Add a NULL check to avoid potential NULL pointer
dereference in subsequent code.

> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -184,6 +184,8 @@ bool seg6_hmac_validate_skb(struct sk_buff *skb)
>  	int require_hmac;
>
>  	idev = __in6_dev_get(skb->dev);
> +	if (!idev)
> +		return false;
>
>  	srh = (struct ipv6_sr_hdr *)skb_transport_header(skb);
>

The patch adds a NULL check in seg6_hmac_validate_skb() to handle the
case when __in6_dev_get() returns NULL. However, does this fix the
complete issue?

Looking at ipv6_srh_rcv() in net/ipv6/exthdrs.c, there's an earlier use
of the same __in6_dev_get() call:

net/ipv6/exthdrs.c:ipv6_srh_rcv() {
    ...
    idev = __in6_dev_get(skb->dev);

    accept_seg6 = min(READ_ONCE(net->ipv6.devconf_all->seg6_enabled),
                      READ_ONCE(idev->cnf.seg6_enabled));
    ...
#ifdef CONFIG_IPV6_SEG6_HMAC
    if (!seg6_hmac_validate_skb(skb)) {
    ...
}

The idev pointer is dereferenced on line 384 (idev->cnf.seg6_enabled)
before seg6_hmac_validate_skb() is called. If __in6_dev_get() can return
NULL as the commit message states, wouldn't this cause a NULL pointer
dereference before the new check in seg6_hmac_validate_skb() has a
chance to catch it?

Should ipv6_srh_rcv() also check for NULL after calling __in6_dev_get()?