| Message ID | 20240405114939.188821-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 86d43e2bf93ccac88ef71cee36a23282ebd9e427 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] af_packet: avoid a false positive warning in packet_setsockopt() | expand |
On Fri, Apr 05, 2024 at 11:49:39AM +0000, Eric Dumazet wrote: > Although the code is correct, the following line > > copy_from_sockptr(&req_u.req, optval, len)); > > triggers this warning : > > memcpy: detected field-spanning write (size 28) of single field "dst" at include/linux/sockptr.h:49 (size 16) > > Refactor the code to be more explicit. > > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Looks good; thanks for making this more clear for the compiler. :) Reviewed-by: Kees Cook <keescook@chromium.org>
Eric Dumazet wrote: > Although the code is correct, the following line > > copy_from_sockptr(&req_u.req, optval, len)); > > triggers this warning : > > memcpy: detected field-spanning write (size 28) of single field "dst" at include/linux/sockptr.h:49 (size 16) > > Refactor the code to be more explicit. > > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Reviewed-by: Willem de Bruijn <willemb@google.com>
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Fri, 5 Apr 2024 11:49:39 +0000 you wrote: > Although the code is correct, the following line > > copy_from_sockptr(&req_u.req, optval, len)); > > triggers this warning : > > memcpy: detected field-spanning write (size 28) of single field "dst" at include/linux/sockptr.h:49 (size 16) > > [...] Here is the summary with links: - [net-next] af_packet: avoid a false positive warning in packet_setsockopt() https://git.kernel.org/netdev/net-next/c/86d43e2bf93c You are awesome, thank you!
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 18f616f487eaad0f7b31fb074e194c0479f30d77..8c6d3fbb4ed87f17c2e365810106a05fe9b8ff0c 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3800,28 +3800,30 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, case PACKET_TX_RING: { union tpacket_req_u req_u; - int len; + ret = -EINVAL; lock_sock(sk); switch (po->tp_version) { case TPACKET_V1: case TPACKET_V2: - len = sizeof(req_u.req); + if (optlen < sizeof(req_u.req)) + break; + ret = copy_from_sockptr(&req_u.req, optval, + sizeof(req_u.req)) ? + -EINVAL : 0; break; case TPACKET_V3: default: - len = sizeof(req_u.req3); + if (optlen < sizeof(req_u.req3)) + break; + ret = copy_from_sockptr(&req_u.req3, optval, + sizeof(req_u.req3)) ? + -EINVAL : 0; break; } - if (optlen < len) { - ret = -EINVAL; - } else { - if (copy_from_sockptr(&req_u.req, optval, len)) - ret = -EFAULT; - else - ret = packet_set_ring(sk, &req_u, 0, - optname == PACKET_TX_RING); - } + if (!ret) + ret = packet_set_ring(sk, &req_u, 0, + optname == PACKET_TX_RING); release_sock(sk); return ret; }
Although the code is correct, the following line copy_from_sockptr(&req_u.req, optval, len)); triggers this warning : memcpy: detected field-spanning write (size 28) of single field "dst" at include/linux/sockptr.h:49 (size 16) Refactor the code to be more explicit. Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Kees Cook <keescook@chromium.org> Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> --- net/packet/af_packet.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)