[PATCH v1 net] calipso: Fix null-ptr-deref in calipso_req_{set, del}attr().

[Paul Moore]

On Mon, Jun 16, 2025 at 1:26 PM Kuniyuki Iwashima <kuni1840 at gmail.com> wrote:
>
> From: Kuniyuki Iwashima <kuniyu at google.com>
>
> syzkaller reported a null-ptr-deref in sock_omalloc() while allocating
> a CALIPSO option.  [0]
>
> The NULL is of struct sock, which was fetched by sk_to_full_sk() in
> calipso_req_setattr().
>
> Since commit a1a5344ddbe8 ("tcp: avoid two atomic ops for syncookies"),
> reqsk->rsk_listener could be NULL when SYN Cookie is returned to its
> client, as hinted by the leading SYN Cookie log.
>
> Here are 3 options to fix the bug:
>
>   1) Return 0 in calipso_req_setattr()
>   2) Return an error in calipso_req_setattr()
>   3) Alaways set rsk_listener
>
> 1) is no go as it bypasses LSM, but 2) effectively disables SYN Cookie
> for CALIPSO.  3) is also no go as there have been many efforts to reduce
> atomic ops and make TCP robust against DDoS.  See also commit 3b24d854cb35
> ("tcp/dccp: do not touch listener sk_refcnt under synflood").
>
> As of the blamed commit, SYN Cookie already did not need refcounting,
> and no one has stumbled on the bug for 9 years, so no CALIPSO user will
> care about SYN Cookie.
>
> Let's return an error in calipso_req_setattr() and calipso_req_delattr()
> in the SYN Cookie case.

I think that's reasonable, but I think it would be nice to have a
quick comment right before the '!sk' checks to help people who may hit
the CALIPSO/SYN-cookie issue in the future.  Maybe "/*
tcp_syncookies=2 can result in sk == NULL */" ?

> diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
> index 62618a058b8f..e25ed02a54bf 100644
> --- a/net/ipv6/calipso.c
> +++ b/net/ipv6/calipso.c
> @@ -1207,6 +1207,9 @@ static int calipso_req_setattr(struct request_sock *req,
>         struct ipv6_opt_hdr *old, *new;
>         struct sock *sk = sk_to_full_sk(req_to_sk(req));
>
> +       if (!sk)
> +               return -ENOMEM;
> +
>         if (req_inet->ipv6_opt && req_inet->ipv6_opt->hopopt)
>                 old = req_inet->ipv6_opt->hopopt;
>         else
> @@ -1247,6 +1250,9 @@ static void calipso_req_delattr(struct request_sock *req)
>         struct ipv6_txoptions *txopts;
>         struct sock *sk = sk_to_full_sk(req_to_sk(req));
>
> +       if (!sk)
> +               return;
> +
>         if (!req_inet->ipv6_opt || !req_inet->ipv6_opt->hopopt)
>                 return;
>
> --
> 2.49.0

-- 
paul-moore.com