Re: [PATCH] netlabel: Fix NULL pointer exception caused by CALIPSO on IPv4 sockets

[Paul Moore]

On Wed, Mar 26, 2025 at 3:44 AM Debin Zhu <mowenroot@xxxxxxx> wrote:
>
> Added IPv6 socket checks in `calipso_sock_getattr`, `calipso_sock_setattr`,
> and `calipso_sock_delattr` functions.
> Return `-EAFNOSUPPORT` error code if the socket is not of the IPv6 type.
> This fix prevents the IPv6 datagram code from
> incorrectly calling the IPv4 datagram code,
> thereby avoiding a NULL pointer exception.
>
> Signed-off-by: Debin Zhu <mowenroot@xxxxxxx>
> Signed-off-by: Bitao Ouyang <1985755126@xxxxxx>
> ---
>  net/ipv6/calipso.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)

Adding netdev and Jakub to the To/CC line, original lore link below:

https://lore.kernel.org/all/20250326074355.24016-1-mowenroot@xxxxxxx/

> diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
> index dbcea9fee..ef55e4176 100644
> --- a/net/ipv6/calipso.c
> +++ b/net/ipv6/calipso.c
> @@ -1072,8 +1072,13 @@ static int calipso_sock_getattr(struct sock *sk,
>         struct ipv6_opt_hdr *hop;
>         int opt_len, len, ret_val = -ENOMSG, offset;
>         unsigned char *opt;
> -       struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk));
> -
> +       struct ipv6_pinfo *pinfo = inet6_sk(sk);
> +       struct ipv6_txoptions *txopts;
> +       /* Prevent IPv6 datagram code from calling IPv4 datagram code, causing pinet6 to be NULL  */
> +       if (!pinfo)
> +               return -EAFNOSUPPORT;
> +
> +       txopts = txopt_get(pinfo);
>         if (!txopts || !txopts->hopopt)
>                 goto done;

For all three function, I'd probably add a single blank line between
the local variable declarations and the code below for the sake of
readability.  I'd probably also drop the comment as the code seems
reasonably obvious (inet6_sk() can return NULL, we can't do anything
with a NULL ptr so bail), but neither are reasons for not applying
this patch, if anything they can be fixed up during the merge assuming
the patch author agrees.

Anyway, this looks good to me, Jakub and/or other netdev folks, we
should get this marked for stable and sent up to Linus, do you want to
do that or should I?

Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx>

> @@ -1125,8 +1130,13 @@ static int calipso_sock_setattr(struct sock *sk,
>  {
>         int ret_val;
>         struct ipv6_opt_hdr *old, *new;
> -       struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk));
> -
> +       struct ipv6_pinfo *pinfo = inet6_sk(sk);
> +       struct ipv6_txoptions *txopts;
> +       /* Prevent IPv6 datagram code from calling IPv4 datagram code, causing pinet6 to be NULL  */
> +       if (!pinfo)
> +               return -EAFNOSUPPORT;
> +
> +       txopts = txopt_get(pinfo);
>         old = NULL;
>         if (txopts)
>                 old = txopts->hopopt;
> @@ -1153,8 +1163,13 @@ static int calipso_sock_setattr(struct sock *sk,
>  static void calipso_sock_delattr(struct sock *sk)
>  {
>         struct ipv6_opt_hdr *new_hop;
> -       struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk));
> -
> +       struct ipv6_pinfo *pinfo = inet6_sk(sk);
> +       struct ipv6_txoptions *txopts;
> +       /* Prevent IPv6 datagram code from calling IPv4 datagram code, causing pinet6 to be NULL  */
> +       if (!pinfo)
> +               return -EAFNOSUPPORT;
> +
> +       txopts = txopt_get(pinfo);
>         if (!txopts || !txopts->hopopt)
>                 goto done;
>
> --
> 2.34.1

-- 
paul-moore.com