Re: [bug report] net/handshake: convert handshake_nl_accept_doit() to FD_PREPARE()

On Wed, Nov 26, 2025 at 11:31:32AM +0300, Dan Carpenter wrote:
> Hello Christian Brauner,
> 
> Commit 214ab7edf554 ("net/handshake: convert
> handshake_nl_accept_doit() to FD_PREPARE()") from Nov 23, 2025
> (linux-next), leads to the following Smatch static checker warning:
> 
> 	net/handshake/netlink.c:128 handshake_nl_accept_doit()
> 	error: we previously assumed 'req' could be null (see line 109)
> 
> net/handshake/netlink.c
>     90 int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
>     91 {
>     92         struct net *net = sock_net(skb->sk);
>     93         struct handshake_net *hn = handshake_pernet(net);
>     94         struct handshake_req *req = NULL;
>     95         struct socket *sock;
>     96         int class, err;
>     97 
>     98         err = -EOPNOTSUPP;
>     99         if (!hn)
>     100                 goto out_status;
>     101 
>     102         err = -EINVAL;
>     103         if (GENL_REQ_ATTR_CHECK(info, HANDSHAKE_A_ACCEPT_HANDLER_CLASS))
>     104                 goto out_status;
>     105         class = nla_get_u32(info->attrs[HANDSHAKE_A_ACCEPT_HANDLER_CLASS]);
>     106 
>     107         err = -EAGAIN;
>     108         req = handshake_req_next(hn, class);
>     109         if (req) {
> 
> If handshake_req_next() returns NULL
> 
>     110                 sock = req->hr_sk->sk_socket;
>     111 
>     112                 FD_PREPARE(fdf, O_CLOEXEC, sock->file);
>     113                 if (fdf.err) {
>     114                         err = fdf.err;
>     115                         goto out_complete;
>     116                 }
>     117 
>     118                 get_file(sock->file); /* FD_PREPARE() consumes a reference. */
>     119                 err = req->hr_proto->hp_accept(req, info, fd_prepare_fd(fdf));
>     120                 if (err)
>     121                         goto out_complete; /* Automatic cleanup handles fput */
>     122 
>     123                 trace_handshake_cmd_accept(net, req, req->hr_sk, fd_prepare_fd(fdf));
>     124                 return fd_publish(fdf);
>     125         }
>     126 
>     127 out_complete:
> --> 128         handshake_complete(req, -EIO, NULL);
>                                    ^^^
> then this will crash.
> 
>     129 out_status:
>     130         trace_handshake_cmd_accept_err(net, req, NULL, err);
>     131         return err;
>     132 }
> 
> regards,
> dan carpenter
> 

Following up with a review comment or two, based on:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/net/handshake/netlink.c?id=0f3d7fa5007a11d8a0eab88c970086363afb1564

The original code followed the preferred early exit / early return
kernel style. The new version inverts this by wrapping the success
path inside:

  if (req) { ... }

instead of checking

  if (!req)
    goto out;

and then continuing the main execution flow at the base indentation
level. Is there a reason the original style needs to be abandoned?

-- 
Chuck Lever