[bug report] io_uring/net: simplify compat selbuf iov parsing

[bug report] io_uring/net: simplify compat selbuf iov parsing

[Dan Carpenter]

Hello Pavel Begunkov,

Commit c30f89f1d08b ("io_uring/net: simplify compat selbuf iov
parsing") from Feb 26, 2025 (linux-next), leads to the following
Smatch static checker warning:

	io_uring/net.c:255 io_compat_msg_copy_hdr()
	warn: unsigned 'tmp_iov.iov_len' is never less than zero.

io_uring/net.c
    228 static int io_compat_msg_copy_hdr(struct io_kiocb *req,
    229                                   struct io_async_msghdr *iomsg,
    230                                   struct compat_msghdr *msg, int ddir,
    231                                   struct sockaddr __user **save_addr)
    232 {
    233         struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
    234         struct compat_iovec __user *uiov;
    235         int ret;
    236 
    237         if (copy_from_user(msg, sr->umsg_compat, sizeof(*msg)))
    238                 return -EFAULT;
    239 
    240         ret = __get_compat_msghdr(&iomsg->msg, msg, save_addr);
    241         if (ret)
    242                 return ret;
    243 
    244         uiov = compat_ptr(msg->msg_iov);
    245         if (req->flags & REQ_F_BUFFER_SELECT) {
    246                 if (msg->msg_iovlen == 0) {
    247                         sr->len = 0;
    248                 } else if (msg->msg_iovlen > 1) {
    249                         return -EINVAL;
    250                 } else {
    251                         struct compat_iovec tmp_iov;
    252 
    253                         if (copy_from_user(&tmp_iov, uiov, sizeof(tmp_iov)))
    254                                 return -EFAULT;
--> 255                         if (tmp_iov.iov_len < 0)
    256                                 return -EINVAL;

This used to be:

-                       if (clen < 0)
+                       if (tmp_iov.iov_len < 0)

Where clen was compat_ssize_t but now tmp_iov.iov_len is unsigned.

    257                         sr->len = tmp_iov.iov_len;
                                ^^^^^^^
sr->len is an int.  So probably we do want to return -EINVAL for negative
lengths.

    258                 }
    259 
    260                 return 0;
    261         }
    262 
    263         return io_net_import_vec(req, iomsg, (struct iovec __user *)uiov,
    264                                  msg->msg_iovlen, ddir);
    265 }

regards,
dan carpenter

Re: [bug report] io_uring/net: simplify compat selbuf iov parsing

[Pavel Begunkov]

On 2/28/25 09:19, Dan Carpenter wrote:
> Hello Pavel Begunkov,
> 
> Commit c30f89f1d08b ("io_uring/net: simplify compat selbuf iov
> parsing") from Feb 26, 2025 (linux-next), leads to the following
> Smatch static checker warning:

Thanks for the report, it should already be fixed up in the tree.
The check is not needed, it's inconsistent with the !COMPAT path,
and import_ubuf() should verify the len for us.

-- 
Pavel Begunkov