| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <666d7e1a7b72d_1ba35a294a5@willemb.c.googlers.com.notmuch>
Date: Sat, 15 Jun 2024 07:42:18 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: zijianzhang@...edance.com,
netdev@...r.kernel.org
Cc: edumazet@...gle.com,
willemdebruijn.kernel@...il.com,
cong.wang@...edance.com,
xiaochun.lu@...edance.com,
Zijian Zhang <zijianzhang@...edance.com>
Subject: Re: [PATCH net-next v5 2/4] sock: support put_cmsg to userspace in TX
path
zijianzhang@ wrote:
> From: Zijian Zhang <zijianzhang@...edance.com>
>
> Since ____sys_sendmsg creates a kernel copy of msg_control and passes
> that to the callees, put_cmsg will write into this kernel buffer. If
> people want to piggyback some information like timestamps upon returning
> of sendmsg. ____sys_sendmsg will have to copy_to_user to the original buf,
> which is not supported. As a result, users typically have to call recvmsg
> on the ERRMSG_QUEUE of the socket, incurring extra system call overhead.
>
> This commit supports put_cmsg to userspace in TX path by storing user
> msg_control address in a new field in struct msghdr, and adding a new bit
> flag use_msg_control_user_tx to toggle the behavior of put_cmsg. Thus,
> it's possible to piggyback information in the msg_control of sendmsg.
>
> Signed-off-by: Zijian Zhang <zijianzhang@...edance.com>
> Signed-off-by: Xiaochun Lu <xiaochun.lu@...edance.com>
> ---
> include/linux/socket.h | 4 ++++
> net/compat.c | 33 +++++++++++++++++++++++++--------
> net/core/scm.c | 42 ++++++++++++++++++++++++++++++++----------
> net/socket.c | 2 ++
> 4 files changed, 63 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 89d16b90370b..8d3db04f4a39 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -71,9 +71,12 @@ struct msghdr {
> void __user *msg_control_user;
> };
> bool msg_control_is_user : 1;
> + bool use_msg_control_user_tx : 1;
> bool msg_get_inq : 1;/* return INQ after receive */
> unsigned int msg_flags; /* flags on received message */
> + void __user *msg_control_user_tx; /* msg_control_user in TX piggyback path */
> __kernel_size_t msg_controllen; /* ancillary data buffer length */
> + __kernel_size_t msg_controllen_user_tx; /* msg_controllen in TX piggyback path */
> struct kiocb *msg_iocb; /* ptr to iocb for async requests */
> struct ubuf_info *msg_ubuf;
> int (*sg_from_iter)(struct sock *sk, struct sk_buff *skb,
> @@ -391,6 +394,7 @@ struct ucred {
>
> extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
> extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
> diff --git a/net/core/scm.c b/net/core/scm.c
> index 4f6a14babe5a..de70ff1981a1 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -228,25 +228,29 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
> }
> EXPORT_SYMBOL(__scm_send);
>
> -int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
> +static int __put_cmsg(struct msghdr *msg, int level, int type, int len, void *data)
> {
> int cmlen = CMSG_LEN(len);
> + __kernel_size_t msg_controllen;
>
> + msg_controllen = msg->use_msg_control_user_tx ?
> + msg->msg_controllen_user_tx : msg->msg_controllen;
> if (msg->msg_flags & MSG_CMSG_COMPAT)
> return put_cmsg_compat(msg, level, type, len, data);
>
> - if (!msg->msg_control || msg->msg_controllen < sizeof(struct cmsghdr)) {
> + if (!msg->msg_control || msg_controllen < sizeof(struct cmsghdr)) {
> msg->msg_flags |= MSG_CTRUNC;
> return 0; /* XXX: return error? check spec. */
> }
> - if (msg->msg_controllen < cmlen) {
> + if (msg_controllen < cmlen) {
> msg->msg_flags |= MSG_CTRUNC;
> - cmlen = msg->msg_controllen;
> + cmlen = msg_controllen;
> }
>
> - if (msg->msg_control_is_user) {
> - struct cmsghdr __user *cm = msg->msg_control_user;
> + if (msg->use_msg_control_user_tx || msg->msg_control_is_user) {
> + struct cmsghdr __user *cm;
>
> + cm = msg->msg_control_is_user ? msg->msg_control_user : msg->msg_control_user_tx;
> check_object_size(data, cmlen - sizeof(*cm), true);
>
> if (!user_write_access_begin(cm, cmlen))
> @@ -267,12 +271,17 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
> memcpy(CMSG_DATA(cm), data, cmlen - sizeof(*cm));
> }
>
> - cmlen = min(CMSG_SPACE(len), msg->msg_controllen);
> - if (msg->msg_control_is_user)
> + cmlen = min(CMSG_SPACE(len), msg_controllen);
> + if (msg->msg_control_is_user) {
> msg->msg_control_user += cmlen;
> - else
> + msg->msg_controllen -= cmlen;
> + } else if (msg->use_msg_control_user_tx) {
> + msg->msg_control_user_tx += cmlen;
> + msg->msg_controllen_user_tx -= cmlen;
> + } else {
> msg->msg_control += cmlen;
> - msg->msg_controllen -= cmlen;
> + msg->msg_controllen -= cmlen;
> + }
> return 0;
>
> efault_end:
> @@ -280,8 +289,21 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
> efault:
> return -EFAULT;
> }
> +
> +int put_cmsg(struct msghdr *msg, int level, int type, int len, void *data)
> +{
> + msg->use_msg_control_user_tx = false;
> + return __put_cmsg(msg, level, type, len, data);
> +}
> EXPORT_SYMBOL(put_cmsg);
>
> +int put_cmsg_user_tx(struct msghdr *msg, int level, int type, int len, void *data)
> +{
> + msg->use_msg_control_user_tx = true;
> + return __put_cmsg(msg, level, type, len, data);
> +}
> +EXPORT_SYMBOL(put_cmsg_user_tx);
> +
> void put_cmsg_scm_timestamping64(struct msghdr *msg, struct scm_timestamping_internal *tss_internal)
> {
> struct scm_timestamping64 tss;
> diff --git a/net/socket.c b/net/socket.c
> index e416920e9399..2755bc7bef9c 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2561,6 +2561,8 @@ static int ____sys_sendmsg(struct socket *sock, struct msghdr *msg_sys,
> err = -EFAULT;
> if (copy_from_user(ctl_buf, msg_sys->msg_control_user, ctl_len))
> goto out_freectl;
> + msg_sys->msg_control_user_tx = msg_sys->msg_control_user;
> + msg_sys->msg_controllen_user_tx = msg_sys->msg_controllen;
No need for this separate user_tx pointer and put_cmsg_user_tx.
___sys_sendmsg copies the user data to a stack allocated kernel
buffer. All subsequent operations are on this buffer. __put_cmsg
already supports writing to this kernel buffer.
All that is needed is to copy_to_user the buffer on return from
__sock_sendmsg. And only if it should be copied, which the bit in
msghdr can signal.
> msg_sys->msg_control = ctl_buf;
> msg_sys->msg_control_is_user = false;
> }
> --
> 2.20.1
>
Powered by blists - more mailing lists