On 10/02/2023 09:16, Kevin Brodsky wrote:
Commit 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control") introduced the msg_control_user and msg_control_is_user fields in struct msghdr to ensure user pointers are represented as such. It also took care of converting most users of struct msghdr::msg_control where user pointers are involved. It did however missed a number of cases, and some code using> msg_control inappropriately has also appeared in the meantime.
This commit is attempting to complete the split. Most issues are about msg_control being used when in fact a user pointer is stored in the union; msg_control_user is now used instead. An exception is made for null checks, as it should be safe to use msg_control unconditionally for that purpose.
Additionally, a special situation in cmsghdr_from_user_compat_to_kern() is addressed. There the input struct msghdr holds a user pointer (msg_control_user), but a kernel pointer is stored in msg_control when returning. msg_control_is_user is now updated accordingly.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is for upstream, see notes in cover letter.
net/compat.c | 13 +++++++------ net/core/scm.c | 9 ++++++--- net/ipv4/tcp.c | 4 ++--
Looking for additional places this might need updating, possibly worth looking at the following:
1. net/socket.c:____sys_recvmsg - a lot going on here, but I *think* it's fine? some weird pointer math
2. net/ipv6/ipv6_sockglue.c:do_ipv6_setsockopt - it's been updated for do_ipv6_getsockopt but not do_ipv6_setsockopt? - sockptr_t optval.is_kernel contains user/kernel status, looks like something similar to getsockopt is required
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/compat.c b/net/compat.c index f2fcf4204a1d..e10570f549a1 100644 --- a/net/compat.c +++ b/net/compat.c @@ -120,7 +120,7 @@ int get_compat_msghdr(struct msghdr *kmsg, #define CMSG_COMPAT_FIRSTHDR(msg) \ (((msg)->msg_controllen) >= sizeof(struct compat_cmsghdr) ? \
(struct compat_cmsghdr __user *)((msg)->msg_control) : \
(struct compat_cmsghdr __user *)((msg)->msg_control_user) : \#define CMSG_COMPAT_OK(ucmlen, ucmsg, mhdr) \ @@ -133,7 +133,7 @@ static inline struct compat_cmsghdr __user *cmsg_compat_nxthdr(struct msghdr *ms struct compat_cmsghdr __user *cmsg, int cmsg_len) { char __user *ptr = (char __user *)cmsg + CMSG_COMPAT_ALIGN(cmsg_len);
- if ((unsigned long)(ptr + 1 - (char __user *)msg->msg_control) >
- if ((unsigned long)(ptr + 1 - (char __user *)msg->msg_control_user) > msg->msg_controllen) return NULL; return (struct compat_cmsghdr __user *)ptr;
@@ -218,6 +218,7 @@ int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg, struct sock *sk, goto Einval; /* Ok, looks like we made it. Hook it up and return success. */
- kmsg->msg_control_is_user = false; kmsg->msg_control = kcmsg_base; kmsg->msg_controllen = kcmlen; return 0;
@@ -232,7 +233,7 @@ int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg, struct sock *sk, int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *data) {
- struct compat_cmsghdr __user *cm = (struct compat_cmsghdr __user *) kmsg->msg_control;
- struct compat_cmsghdr __user *cm = (struct compat_cmsghdr __user *) kmsg->msg_control_user; struct compat_cmsghdr cmhdr; struct old_timeval32 ctv; struct old_timespec32 cts[3];
@@ -281,7 +282,7 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat cmlen = CMSG_COMPAT_SPACE(len); if (kmsg->msg_controllen < cmlen) cmlen = kmsg->msg_controllen;
- kmsg->msg_control += cmlen;
- kmsg->msg_control_user += cmlen; kmsg->msg_controllen -= cmlen; return 0;
} @@ -296,7 +297,7 @@ static int scm_max_fds_compat(struct msghdr *msg) void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) { struct compat_cmsghdr __user *cm =
(struct compat_cmsghdr __user *)msg->msg_control;
(struct compat_cmsghdr __user *)msg->msg_control_user;@@ -320,7 +321,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) cmlen = CMSG_COMPAT_SPACE(i * sizeof(int)); if (msg->msg_controllen < cmlen) cmlen = msg->msg_controllen;
msg->msg_control += cmlen;
msg->msg_control_user += cmlen; msg->msg_controllen -= cmlen;diff --git a/net/core/scm.c b/net/core/scm.c index 5c356f0dee30..cb9dc17f1139 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -248,7 +248,10 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data) } cmlen = min(CMSG_SPACE(len), msg->msg_controllen);
- msg->msg_control += cmlen;
- if (msg->msg_control_is_user)
msg->msg_control_user += cmlen;- else
msg->msg_control += cmlen;@@ -297,7 +300,7 @@ static int scm_max_fds(struct msghdr *msg) void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) { struct cmsghdr __user *cm =
(__force struct cmsghdr __user *)msg->msg_control;
(__force struct cmsghdr __user *)msg->msg_control_user;@@ -330,7 +333,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) cmlen = CMSG_SPACE(i * sizeof(int)); if (msg->msg_controllen < cmlen) cmlen = msg->msg_controllen;
msg->msg_control += cmlen;
msg->msg_control_user += cmlen; msg->msg_controllen -= cmlen;diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 17b213acffc8..8f5e605ea673 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -2163,7 +2163,7 @@ static void tcp_zc_finalize_rx_tstamp(struct sock *sk, struct msghdr cmsg_dummy; msg_control_addr = (unsigned long)zc->msg_control;
- cmsg_dummy.msg_control = (void *)msg_control_addr;
- cmsg_dummy.msg_control_user = (void __user *)msg_control_addr; cmsg_dummy.msg_controllen = (__kernel_size_t)zc->msg_controllen; cmsg_dummy.msg_flags = in_compat_syscall()
@@ -2174,7 +2174,7 @@ static void tcp_zc_finalize_rx_tstamp(struct sock *sk, zc->msg_controllen == cmsg_dummy.msg_controllen) { tcp_recv_timestamp(&cmsg_dummy, sk, tss); zc->msg_control = (__u64)
((uintptr_t)cmsg_dummy.msg_control);
((uintptr_t)cmsg_dummy.msg_control_user);