Commit 18912c52 authored by Stanislav Fomichev's avatar Stanislav Fomichev Committed by Jakub Kicinski
Browse files

tcp: devmem: don't write truncated dmabuf CMSGs to userspace 



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: 8f0b3cc9 ("tcp: RX path for devmem TCP")
Reviewed-by: default avatarMina Almasry <almasrymina@google.com>
Signed-off-by: default avatarStanislav Fomichev <sdf@fomichev.me>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20250224174401.3582695-1-sdf@fomichev.me


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent bab3a6e9

include/linux/socket.h

+2 −0
Original line number Diff line number Diff line
@@ -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;

net/core/scm.c

+10 −0
Original line number Diff line number Diff line
@@ -282,6 +282,16 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data) 
}

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;

net/ipv4/tcp.c

+10 −16
Original line number Diff line number Diff line
@@ -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,

				err = put_cmsg_notrunc(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;

				if (err)

					goto out;

				}



				atomic_long_inc(&niov->pp_ref_count);

				tcp_xa_pool.netmems[tcp_xa_pool.idx++] = skb_frag_netmem(frag);