| Message ID | 20230216124340.875338-1-jakub@cloudflare.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 436864095a95fcc611c20c44a111985fa9848730 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] selftests/net: Interpret UDP_GRO cmsg data as an int value | expand |
On Thu, Feb 16, 2023 at 1:43 PM Jakub Sitnicki <jakub@cloudflare.com> wrote: > > Data passed to user-space with a (SOL_UDP, UDP_GRO) cmsg carries an > int (see udp_cmsg_recv), not a u16 value, as strace confirms: > > recvmsg(8, {msg_name=..., > msg_iov=[{iov_base="\0\0..."..., iov_len=96000}], > msg_iovlen=1, > msg_control=[{cmsg_len=20, <-- sizeof(cmsghdr) + 4 > cmsg_level=SOL_UDP, > cmsg_type=0x68}], <-- UDP_GRO > msg_controllen=24, > msg_flags=0}, 0) = 11200 > > Interpreting the data as an u16 value won't work on big-endian platforms. > Since it is too late to back out of this API decision [1], fix the test. > > [1]: https://lore.kernel.org/netdev/20230131174601.203127-1-jakub@cloudflare.com/ > > Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO") > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > --- > tools/testing/selftests/net/udpgso_bench_rx.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c > index 4058c7451e70..f35a924d4a30 100644 > --- a/tools/testing/selftests/net/udpgso_bench_rx.c > +++ b/tools/testing/selftests/net/udpgso_bench_rx.c > @@ -214,11 +214,10 @@ static void do_verify_udp(const char *data, int len) > > static int recv_msg(int fd, char *buf, int len, int *gso_size) > { > - char control[CMSG_SPACE(sizeof(uint16_t))] = {0}; > + char control[CMSG_SPACE(sizeof(int))] = {0}; > struct msghdr msg = {0}; > struct iovec iov = {0}; > struct cmsghdr *cmsg; > - uint16_t *gsosizeptr; > int ret; > > iov.iov_base = buf; > @@ -237,8 +236,7 @@ static int recv_msg(int fd, char *buf, int len, int *gso_size) > cmsg = CMSG_NXTHDR(&msg, cmsg)) { > if (cmsg->cmsg_level == SOL_UDP > && cmsg->cmsg_type == UDP_GRO) { > - gsosizeptr = (uint16_t *) CMSG_DATA(cmsg); > - *gso_size = *gsosizeptr; > + *gso_size = *(int *)CMSG_DATA(cmsg); > break; > } > } > -- > 2.39.1 > Yes, it seems user space often do not bother verifying cmsg->cmsg_len against CMSG_LEN(sizeof(expected type)) (such sanity tests could be added to many of our selftests) Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks.
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Thu, 16 Feb 2023 13:43:40 +0100 you wrote: > Data passed to user-space with a (SOL_UDP, UDP_GRO) cmsg carries an > int (see udp_cmsg_recv), not a u16 value, as strace confirms: > > recvmsg(8, {msg_name=..., > msg_iov=[{iov_base="\0\0..."..., iov_len=96000}], > msg_iovlen=1, > msg_control=[{cmsg_len=20, <-- sizeof(cmsghdr) + 4 > cmsg_level=SOL_UDP, > cmsg_type=0x68}], <-- UDP_GRO > msg_controllen=24, > msg_flags=0}, 0) = 11200 > > [...] Here is the summary with links: - [net] selftests/net: Interpret UDP_GRO cmsg data as an int value https://git.kernel.org/netdev/net/c/436864095a95 You are awesome, thank you!
diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c index 4058c7451e70..f35a924d4a30 100644 --- a/tools/testing/selftests/net/udpgso_bench_rx.c +++ b/tools/testing/selftests/net/udpgso_bench_rx.c @@ -214,11 +214,10 @@ static void do_verify_udp(const char *data, int len) static int recv_msg(int fd, char *buf, int len, int *gso_size) { - char control[CMSG_SPACE(sizeof(uint16_t))] = {0}; + char control[CMSG_SPACE(sizeof(int))] = {0}; struct msghdr msg = {0}; struct iovec iov = {0}; struct cmsghdr *cmsg; - uint16_t *gsosizeptr; int ret; iov.iov_base = buf; @@ -237,8 +236,7 @@ static int recv_msg(int fd, char *buf, int len, int *gso_size) cmsg = CMSG_NXTHDR(&msg, cmsg)) { if (cmsg->cmsg_level == SOL_UDP && cmsg->cmsg_type == UDP_GRO) { - gsosizeptr = (uint16_t *) CMSG_DATA(cmsg); - *gso_size = *gsosizeptr; + *gso_size = *(int *)CMSG_DATA(cmsg); break; } }
Data passed to user-space with a (SOL_UDP, UDP_GRO) cmsg carries an int (see udp_cmsg_recv), not a u16 value, as strace confirms: recvmsg(8, {msg_name=..., msg_iov=[{iov_base="\0\0..."..., iov_len=96000}], msg_iovlen=1, msg_control=[{cmsg_len=20, <-- sizeof(cmsghdr) + 4 cmsg_level=SOL_UDP, cmsg_type=0x68}], <-- UDP_GRO msg_controllen=24, msg_flags=0}, 0) = 11200 Interpreting the data as an u16 value won't work on big-endian platforms. Since it is too late to back out of this API decision [1], fix the test. [1]: https://lore.kernel.org/netdev/20230131174601.203127-1-jakub@cloudflare.com/ Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO") Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- tools/testing/selftests/net/udpgso_bench_rx.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)