Re: [PATCH 1/6] [net] drop 'size' argument of sock_recvmsg()

From: Jeff Layton
Date: Tue Apr 19 2016 - 12:03:40 EST


On Sat, 2016-04-09 at 21:50 +0100, Al Viro wrote:
> all callers have it equal to msg_data_left(msg).
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> Âdrivers/target/iscsi/iscsi_target_util.c |ÂÂ5 ++---
> Âinclude/linux/net.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ3 +--
> Ânet/socket.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 23 ++++++++++-------------
> Â3 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
> index 428b0d9..5772038 100644
> --- a/drivers/target/iscsi/iscsi_target_util.c
> +++ b/drivers/target/iscsi/iscsi_target_util.c
> @@ -1283,9 +1283,8 @@ static int iscsit_do_rx_data(
> Â iov_iter_kvec(&msg.msg_iter, READ | ITER_KVEC,
> Â ÂÂÂÂÂÂcount->iov, count->iov_count, data);
> Â
> - while (total_rx < data) {
> - rx_loop = sock_recvmsg(conn->sock, &msg,
> - ÂÂÂÂÂÂ(data - total_rx), MSG_WAITALL);
> + while (msg_data_left(&msg)) {
> + rx_loop = sock_recvmsg(conn->sock, &msg, MSG_WAITALL);
> Â if (rx_loop <= 0) {
> Â pr_debug("rx_loop: %d total_rx: %d\n",
> Â rx_loop, total_rx);
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 49175e4..72c1e06 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -218,8 +218,7 @@ int sock_create_lite(int family, int type, int proto, struct socket **res);
> Âstruct socket *sock_alloc(void);
> Âvoid sock_release(struct socket *sock);
> Âint sock_sendmsg(struct socket *sock, struct msghdr *msg);
> -int sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
> - Âint flags);
> +int sock_recvmsg(struct socket *sock, struct msghdr *msg, int flags);
> Âstruct file *sock_alloc_file(struct socket *sock, int flags, const char *dname);
> Âstruct socket *sockfd_lookup(int fd, int *err);
> Âstruct socket *sock_from_file(struct file *file, int *err);
> diff --git a/net/socket.c b/net/socket.c
> index 5f77a8e..956426e3 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -709,17 +709,16 @@ void __sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
> ÂEXPORT_SYMBOL_GPL(__sock_recv_ts_and_drops);
> Â
> Âstatic inline int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg,
> - ÂÂÂÂÂsize_t size, int flags)
> + ÂÂÂÂÂint flags)
> Â{
> - return sock->ops->recvmsg(sock, msg, size, flags);
> + return sock->ops->recvmsg(sock, msg, msg_data_left(msg), flags);
> Â}
> Â
> -int sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
> - Âint flags)
> +int sock_recvmsg(struct socket *sock, struct msghdr *msg, int flags)
> Â{
> - int err = security_socket_recvmsg(sock, msg, size, flags);
> + int err = security_socket_recvmsg(sock, msg, msg_data_left(msg), flags);
> Â
> - return err ?: sock_recvmsg_nosec(sock, msg, size, flags);
> + return err ?: sock_recvmsg_nosec(sock, msg, flags);
> Â}
> ÂEXPORT_SYMBOL(sock_recvmsg);
> Â
> @@ -746,7 +745,7 @@ int kernel_recvmsg(struct socket *sock, struct msghdr *msg,
> Â
> Â iov_iter_kvec(&msg->msg_iter, READ | ITER_KVEC, vec, num, size);
> Â set_fs(KERNEL_DS);
> - result = sock_recvmsg(sock, msg, size, flags);
> + result = sock_recvmsg(sock, msg, flags);
> Â set_fs(oldfs);
> Â return result;
> Â}
> @@ -796,7 +795,7 @@ static ssize_t sock_read_iter(struct kiocb *iocb, struct iov_iter *to)
> Â if (!iov_iter_count(to)) /* Match SYS5 behaviour */
> Â return 0;
> Â
> - res = sock_recvmsg(sock, &msg, iov_iter_count(to), msg.msg_flags);
> + res = sock_recvmsg(sock, &msg, msg.msg_flags);
> Â *to = msg.msg_iter;
> Â return res;
> Â}
> @@ -1696,7 +1695,7 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
> Â msg.msg_iocb = NULL;
> Â if (sock->file->f_flags & O_NONBLOCK)
> Â flags |= MSG_DONTWAIT;
> - err = sock_recvmsg(sock, &msg, iov_iter_count(&msg.msg_iter), flags);
> + err = sock_recvmsg(sock, &msg, flags);
> Â
> Â if (err >= 0 && addr != NULL) {
> Â err2 = move_addr_to_user(&address,
> @@ -2073,7 +2072,7 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
> Â struct iovec iovstack[UIO_FASTIOV];
> Â struct iovec *iov = iovstack;
> Â unsigned long cmsg_ptr;
> - int total_len, len;
> + int len;
> Â ssize_t err;
> Â
> Â /* kernel mode address */
> @@ -2091,7 +2090,6 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
> Â err = copy_msghdr_from_user(msg_sys, msg, &uaddr, &iov);
> Â if (err < 0)
> Â return err;
> - total_len = iov_iter_count(&msg_sys->msg_iter);
> Â
> Â cmsg_ptr = (unsigned long)msg_sys->msg_control;
> Â msg_sys->msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
> @@ -2101,8 +2099,7 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
> Â
> Â if (sock->file->f_flags & O_NONBLOCK)
> Â flags |= MSG_DONTWAIT;
> - err = (nosec ? sock_recvmsg_nosec : sock_recvmsg)(sock, msg_sys,
> - ÂÂtotal_len, flags);
> + err = (nosec ? sock_recvmsg_nosec : sock_recvmsg)(sock, msg_sys, flags);
> Â if (err < 0)
> Â goto out_freeiov;
> Â len = err;

Looks good. Assuming that the corresponding change to sock_recvmsg is
merged.

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>