linux-kernel - Re: [PATCH] net: 9p: Fix a possible null-pointer dereference in p9_cm_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADm8TenzjtK7pHVJnVuxjK=rWAumhY7oZf+yoZ7OV6aSdd+2gg@mail.gmail.com>
Date: Thu, 18 Dec 2025 21:13:12 +0800
From: Tuo Li <islituo@...il.com>
To: asmadeus@...ewreck.org
Cc: ericvh@...nel.org, lucho@...kov.net, linux_oss@...debyte.com, 
	v9fs@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: 9p: Fix a possible null-pointer dereference in p9_cm_event_handler()

On Thu, Dec 18, 2025 at 8:56 PM <asmadeus@...ewreck.org> wrote:
>
> Tuo Li wrote on Thu, Dec 18, 2025 at 03:10:12PM +0800:
> > In p9_cm_event_handler(), rdma is checked in the RDMA_CM_EVENT_DISCONNECTED
> > case, indicating that it may be NULL. If this happens, a null-pointer
> > dereference can occur when complete() is called:
> >
> >   complete(&rdma->cm_done);
> >
> > To prevent such a potential null-pointer dereference, add a defensive check
> > before invoking complete().
>
> In practice it doesn't really make sense for rdma to be NULL here
> though -- and we can make sure of it by assigning client->trans before
> creating the cm_id and get rid of the other null check instead:
> -----
> diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
> index b5ceae1cccbb..f2d81f3ef255 100644
> --- a/net/9p/trans_rdma.c
> +++ b/net/9p/trans_rdma.c
> @@ -145,8 +145,7 @@ p9_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
>                 break;
>
>         case RDMA_CM_EVENT_DISCONNECTED:
> -               if (rdma)
> -                       rdma->state = P9_RDMA_CLOSED;
> +               rdma->state = P9_RDMA_CLOSED;
>                 c->status = Disconnected;
>                 break;
>
> @@ -541,15 +540,15 @@ rdma_create_trans(struct p9_client *client, struct fs_context *fc)
>         if (!rdma)
>                 return -ENOMEM;
>
> +       /* Associate the client with the transport */
> +       client->trans = rdma;
> +
>         /* Create the RDMA CM ID */
>         rdma->cm_id = rdma_create_id(&init_net, p9_cm_event_handler, client,
>                                     RDMA_PS_TCP, IB_QPT_RC);
>         if (IS_ERR(rdma->cm_id))
>                 goto error;
>
> -       /* Associate the client with the transport */
> -       client->trans = rdma;
> -
>         /* Bind to a privileged port if we need to */
>         if (opts.privport) {
>                 err = p9_rdma_bind_privport(rdma);
> -----
>
> What do you think?
> --
> Dominique

Thanks for the clarification.
It indeed makes no sense for rdma to be NULL here.
The updated fix looks reasonable to me.

Sincerely,
Tuo Li