On 2/7/21 10:49 AM, David Ahern wrote: > On 2/6/21 1:36 PM, Arjun Roy wrote: >> From: Arjun Roy <arjun...@google.com> >> >> Explicitly define reserved field and require it to be 0-valued. >> >> Fixes: 7eeba1706eba ("tcp: Add receive timestamp support for receive >> zerocopy.") >> Signed-off-by: Arjun Roy <arjun...@google.com> >> Signed-off-by: Eric Dumazet <eduma...@google.com> >> Signed-off-by: Soheil Hassas Yeganeh <soh...@google.com> >> Suggested-by: David Ahern <dsah...@gmail.com> >> Suggested-by: Leon Romanovsky <l...@kernel.org> >> Suggested-by: Jakub Kicinski <k...@kernel.org> >> --- >> include/uapi/linux/tcp.h | 2 +- >> net/ipv4/tcp.c | 2 ++ >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h >> index 42fc5a640df4..8fc09e8638b3 100644 >> --- a/include/uapi/linux/tcp.h >> +++ b/include/uapi/linux/tcp.h >> @@ -357,6 +357,6 @@ struct tcp_zerocopy_receive { >> __u64 msg_control; /* ancillary data */ >> __u64 msg_controllen; >> __u32 msg_flags; >> - /* __u32 hole; Next we must add >1 u32 otherwise length checks fail. */ >> + __u32 reserved; /* set to 0 for now */ >> }; >> #endif /* _UAPI_LINUX_TCP_H */ >> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c >> index e1a17c6b473c..c8469c579ed8 100644 >> --- a/net/ipv4/tcp.c >> +++ b/net/ipv4/tcp.c >> @@ -4159,6 +4159,8 @@ static int do_tcp_getsockopt(struct sock *sk, int >> level, >> } >> if (copy_from_user(&zc, optval, len)) >> return -EFAULT; >> + if (zc.reserved) >> + return -EINVAL; >> lock_sock(sk); >> err = tcp_zerocopy_receive(sk, &zc, &tss); >> release_sock(sk); >> > > > The 'switch (len)' statement needs to be updated now that 'len' is not > going to end on the 'msg_flags' boundary? But then, how did that work > before if there was 4 byte padding? > > Maybe I am missing something here. You currently have: > > switch (len) { > case offsetofend(struct tcp_zerocopy_receive, msg_flags): >
Ah, I missed the lines before it: if (len >= offsetofend(struct tcp_zerocopy_receive, msg_flags)) goto zerocopy_rcv_cmsg; Also, I see a check on zc.msg_flags for a specific flag option being set. What about other invalid bits in msg_flags? I do not see a check like: #define TCP_VALID_ZC_MSG_FLAGS (TCP_CMSG_TS) if (zc.msg_flags & ~(TCP_VALID_ZC_MSG_FLAGS)) return -EINVAL;