tcp: devmem: don't write truncated dmabuf CMSGs to userspace
authorStanislav Fomichev <sdf@fomichev.me>
Mon, 24 Feb 2025 17:44:01 +0000 (09:44 -0800)
committerJakub Kicinski <kuba@kernel.org>
Wed, 26 Feb 2025 02:15:43 +0000 (18:15 -0800)
Currently, we report -ETOOSMALL (err) only on the first iteration
(!sent). When we get put_cmsg error after a bunch of successful
put_cmsg calls, we don't signal the error at all. This might be
confusing on the userspace side which will see truncated CMSGs
but no MSG_CTRUNC signal.

Consider the following case:
- sizeof(struct cmsghdr) = 16
- sizeof(struct dmabuf_cmsg) = 24
- total cmsg size (CMSG_LEN) = 40 (16+24)

When calling recvmsg with msg_controllen=60, the userspace
will receive two(!) dmabuf_cmsg(s), the first one will
be a valid one and the second one will be silently truncated. There is no
easy way to discover the truncation besides doing something like
"cm->cmsg_len != CMSG_LEN(sizeof(dmabuf_cmsg))".

Introduce new put_devmem_cmsg wrapper that reports an error instead
of doing the truncation. Mina suggests that it's the intended way
this API should work.

Note that we might now report MSG_CTRUNC when the users (incorrectly)
call us with msg_control == NULL.

Fixes: 8f0b3cc9a4c1 ("tcp: RX path for devmem TCP")
Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20250224174401.3582695-1-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
include/linux/socket.h
net/core/scm.c
net/ipv4/tcp.c

index d18cc47e89bd0164ec8de7702c266a406a2c4647..c3322eb3d6865d5ef90ea7891d2aa98d3efe4f6d 100644 (file)
@@ -392,6 +392,8 @@ struct ucred {
 
 extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
 extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
+extern int put_cmsg_notrunc(struct msghdr *msg, int level, int type, int len,
+                           void *data);
 
 struct timespec64;
 struct __kernel_timespec;
index 4f6a14babe5ae36aa0be3f343a3ceafaeacbf25d..733c0cbd393d241b530aadc292de84af25225122 100644 (file)
@@ -282,6 +282,16 @@ efault:
 }
 EXPORT_SYMBOL(put_cmsg);
 
+int put_cmsg_notrunc(struct msghdr *msg, int level, int type, int len,
+                    void *data)
+{
+       /* Don't produce truncated CMSGs */
+       if (!msg->msg_control || msg->msg_controllen < CMSG_LEN(len))
+               return -ETOOSMALL;
+
+       return put_cmsg(msg, level, type, len, data);
+}
+
 void put_cmsg_scm_timestamping64(struct msghdr *msg, struct scm_timestamping_internal *tss_internal)
 {
        struct scm_timestamping64 tss;
index 0d704bda6c416bd722223eb19bec5667df4e7bb7..d74281eca14f0b7fb8f41c85856a45e3fa973e3d 100644 (file)
@@ -2438,14 +2438,12 @@ static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb,
                         */
                        memset(&dmabuf_cmsg, 0, sizeof(dmabuf_cmsg));
                        dmabuf_cmsg.frag_size = copy;
-                       err = put_cmsg(msg, SOL_SOCKET, SO_DEVMEM_LINEAR,
-                                      sizeof(dmabuf_cmsg), &dmabuf_cmsg);
-                       if (err || msg->msg_flags & MSG_CTRUNC) {
-                               msg->msg_flags &= ~MSG_CTRUNC;
-                               if (!err)
-                                       err = -ETOOSMALL;
+                       err = put_cmsg_notrunc(msg, SOL_SOCKET,
+                                              SO_DEVMEM_LINEAR,
+                                              sizeof(dmabuf_cmsg),
+                                              &dmabuf_cmsg);
+                       if (err)
                                goto out;
-                       }
 
                        sent += copy;
 
@@ -2499,16 +2497,12 @@ static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb,
                                offset += copy;
                                remaining_len -= copy;
 
-                               err = put_cmsg(msg, SOL_SOCKET,
-                                              SO_DEVMEM_DMABUF,
-                                              sizeof(dmabuf_cmsg),
-                                              &dmabuf_cmsg);
-                               if (err || msg->msg_flags & MSG_CTRUNC) {
-                                       msg->msg_flags &= ~MSG_CTRUNC;
-                                       if (!err)
-                                               err = -ETOOSMALL;
+                               err = put_cmsg_notrunc(msg, SOL_SOCKET,
+                                                      SO_DEVMEM_DMABUF,
+                                                      sizeof(dmabuf_cmsg),
+                                                      &dmabuf_cmsg);
+                               if (err)
                                        goto out;
-                               }
 
                                atomic_long_inc(&niov->pp_ref_count);
                                tcp_xa_pool.netmems[tcp_xa_pool.idx++] = skb_frag_netmem(frag);