forked from mirrors/linux
		
	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: 8f0b3cc9a4 ("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>
			
			
This commit is contained in:
		
							parent
							
								
									bab3a6e9ff
								
							
						
					
					
						commit
						18912c5206
					
				
					 3 changed files with 22 additions and 16 deletions
				
			
		|  | @ -392,6 +392,8 @@ struct ucred { | ||||||
| 
 | 
 | ||||||
| extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr); | 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(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 timespec64; | ||||||
| struct __kernel_timespec; | struct __kernel_timespec; | ||||||
|  |  | ||||||
|  | @ -282,6 +282,16 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data) | ||||||
| } | } | ||||||
| EXPORT_SYMBOL(put_cmsg); | 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) | void put_cmsg_scm_timestamping64(struct msghdr *msg, struct scm_timestamping_internal *tss_internal) | ||||||
| { | { | ||||||
| 	struct scm_timestamping64 tss; | 	struct scm_timestamping64 tss; | ||||||
|  |  | ||||||
|  | @ -2438,14 +2438,12 @@ static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb, | ||||||
| 			 */ | 			 */ | ||||||
| 			memset(&dmabuf_cmsg, 0, sizeof(dmabuf_cmsg)); | 			memset(&dmabuf_cmsg, 0, sizeof(dmabuf_cmsg)); | ||||||
| 			dmabuf_cmsg.frag_size = copy; | 			dmabuf_cmsg.frag_size = copy; | ||||||
| 			err = put_cmsg(msg, SOL_SOCKET, SO_DEVMEM_LINEAR, | 			err = put_cmsg_notrunc(msg, SOL_SOCKET, | ||||||
| 				       sizeof(dmabuf_cmsg), &dmabuf_cmsg); | 					       SO_DEVMEM_LINEAR, | ||||||
| 			if (err || msg->msg_flags & MSG_CTRUNC) { | 					       sizeof(dmabuf_cmsg), | ||||||
| 				msg->msg_flags &= ~MSG_CTRUNC; | 					       &dmabuf_cmsg); | ||||||
| 				if (!err) | 			if (err) | ||||||
| 					err = -ETOOSMALL; |  | ||||||
| 				goto out; | 				goto out; | ||||||
| 			} |  | ||||||
| 
 | 
 | ||||||
| 			sent += copy; | 			sent += copy; | ||||||
| 
 | 
 | ||||||
|  | @ -2499,16 +2497,12 @@ static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb, | ||||||
| 				offset += copy; | 				offset += copy; | ||||||
| 				remaining_len -= copy; | 				remaining_len -= copy; | ||||||
| 
 | 
 | ||||||
| 				err = put_cmsg(msg, SOL_SOCKET, | 				err = put_cmsg_notrunc(msg, SOL_SOCKET, | ||||||
| 					       SO_DEVMEM_DMABUF, | 						       SO_DEVMEM_DMABUF, | ||||||
| 					       sizeof(dmabuf_cmsg), | 						       sizeof(dmabuf_cmsg), | ||||||
| 					       &dmabuf_cmsg); | 						       &dmabuf_cmsg); | ||||||
| 				if (err || msg->msg_flags & MSG_CTRUNC) { | 				if (err) | ||||||
| 					msg->msg_flags &= ~MSG_CTRUNC; |  | ||||||
| 					if (!err) |  | ||||||
| 						err = -ETOOSMALL; |  | ||||||
| 					goto out; | 					goto out; | ||||||
| 				} |  | ||||||
| 
 | 
 | ||||||
| 				atomic_long_inc(&niov->pp_ref_count); | 				atomic_long_inc(&niov->pp_ref_count); | ||||||
| 				tcp_xa_pool.netmems[tcp_xa_pool.idx++] = skb_frag_netmem(frag); | 				tcp_xa_pool.netmems[tcp_xa_pool.idx++] = skb_frag_netmem(frag); | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Stanislav Fomichev
						Stanislav Fomichev