diff mbox series

udp: change MSG_TRUNC return behaviour for MSG_PEEK in recvmsg

Message ID YjodjXHN7j69h/kd@kappa (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series udp: change MSG_TRUNC return behaviour for MSG_PEEK in recvmsg | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 1 maintainers not CCed: pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 13 this patch: 13
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Torin Carey March 22, 2022, 7:03 p.m. UTC 
Make UDP recvmsg only return the MSG_TRUNC flag if the read does not
copy the tail end of the datagram.  Specifically, this targets MSG_PEEK
when we're using a positive peek offset.

The current behaviour means that if we have a positive peek offset `off`
and we're reading `r` bytes from a datagram of `ulen` length, we respond
with MSG_TRUNC if and only if `r <= ulen - off`.  This is odd behaviour
as we return MSG_TRUNC if the user requests exactly `ulen - off` which
has no truncation.

The behaviour could be corrected in two ways:

This patch returns MSG_TRUNC only for tail-end truncation and not head
truncation.  This is more consistent with recv(2):
> MSG_TRUNC
>     indicates that the trailing portion of a datagram was discarded
>     because the datagram was larger than the buffer supplied.
although this isn't written with SO_PEEK_OFF in mind.

The second option is to always return MSG_TRUNC if `off > 0` like the
man-pages socket(7) page states:
> For datagram sockets, if the "peek offset" points to the middle of a
> packet, the data returned will be marked with the MSG_TRUNC flag.

Signed-off-by: Torin Carey <torin@tcarey.uk>
---
 net/ipv4/udp.c | 2 +-
 net/ipv6/udp.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
2.34.1

Comments

David Laight March 23, 2022, 1:57 a.m. UTC | #1 
From: Torin Carey
> Sent: 22 March 2022 19:04
> 
> Make UDP recvmsg only return the MSG_TRUNC flag if the read does not
> copy the tail end of the datagram.  Specifically, this targets MSG_PEEK
> when we're using a positive peek offset.
> 
> The current behaviour means that if we have a positive peek offset `off`
> and we're reading `r` bytes from a datagram of `ulen` length, we respond
> with MSG_TRUNC if and only if `r <= ulen - off`.  This is odd behaviour
> as we return MSG_TRUNC if the user requests exactly `ulen - off` which
> has no truncation.
> 
> The behaviour could be corrected in two ways:
> 
> This patch returns MSG_TRUNC only for tail-end truncation and not head
> truncation.  This is more consistent with recv(2):
> > MSG_TRUNC
> >     indicates that the trailing portion of a datagram was discarded
> >     because the datagram was larger than the buffer supplied.
> although this isn't written with SO_PEEK_OFF in mind.
> 
> The second option is to always return MSG_TRUNC if `off > 0` like the
> man-pages socket(7) page states:
> > For datagram sockets, if the "peek offset" points to the middle of a
> > packet, the data returned will be marked with the MSG_TRUNC flag.
> 
> Signed-off-by: Torin Carey <torin@tcarey.uk>
> ---
>  net/ipv4/udp.c | 2 +-
>  net/ipv6/udp.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 319dd7bbfe33..e57740a2c308 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1855,7 +1855,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
>  	copied = len;
>  	if (copied > ulen - off)
>  		copied = ulen - off;
> -	else if (copied < ulen)
> +	else if (copied < ulen - off)
>  		msg->msg_flags |= MSG_TRUNC;

You can remove a test:
	if (copied >= ulen - off)
		copied = ulen - off;
	else
		msg->msg_flags |= MSG_TRUNC;

    David

> 
>  	/*
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 14a94cddcf0b..d6c0eed94564 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -348,7 +348,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	copied = len;
>  	if (copied > ulen - off)
>  		copied = ulen - off;
> -	else if (copied < ulen)
> +	else if (copied < ulen - off)
>  		msg->msg_flags |= MSG_TRUNC;
> 
>  	is_udp4 = (skb->protocol == htons(ETH_P_IP));
> --
> 2.34.1
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 319dd7bbfe33..e57740a2c308 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1855,7 +1855,7 @@  int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
 	copied = len;
 	if (copied > ulen - off)
 		copied = ulen - off;
-	else if (copied < ulen)
+	else if (copied < ulen - off)
 		msg->msg_flags |= MSG_TRUNC;

 	/*
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 14a94cddcf0b..d6c0eed94564 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -348,7 +348,7 @@  int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	copied = len;
 	if (copied > ulen - off)
 		copied = ulen - off;
-	else if (copied < ulen)
+	else if (copied < ulen - off)
 		msg->msg_flags |= MSG_TRUNC;

 	is_udp4 = (skb->protocol == htons(ETH_P_IP));