linux-kernel - Re: [PATCH 1/1] net: ioctl: Use kernel memory on protocol ioctl callbacks

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e5a3b404-b958-c833-1032-70c78e159940@kernel.org>
Date:   Fri, 19 May 2023 21:50:34 -0600
From:   David Ahern <dsahern@...nel.org>
To:     Breno Leitao <leitao@...ian.org>, axboe@...nel.dk,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, willemdebruijn.kernel@...il.com,
        courmisch@...il.com, nhorman@...driver.com
Cc:     asml.silence@...il.com, alex.aring@...il.com, dccp@...r.kernel.org,
        mptcp@...ts.linux.dev, linux-kernel@...r.kernel.org,
        matthieu.baerts@...sares.net, marcelo.leitner@...il.com,
        linux-wpan@...r.kernel.org, linux-sctp@...r.kernel.org,
        leit@...com, David.Laight@...LAB.COM
Subject: Re: [PATCH 1/1] net: ioctl: Use kernel memory on protocol ioctl
 callbacks

On 5/19/23 7:58 AM, Breno Leitao wrote:
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 5440e67bcfe3..47567909baf2 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -4106,3 +4109,107 @@ int sock_bind_add(struct sock *sk, struct sockaddr *addr, int addr_len)
>  	return sk->sk_prot->bind_add(sk, addr, addr_len);
>  }
>  EXPORT_SYMBOL(sock_bind_add);
> +
> +/* Copy 'size' bytes from userspace and do not copy anything back */
> +int sock_skproto_ioctl_in(struct sock *sk, unsigned int cmd,
> +			  void __user *arg)

Unless I missed a reference, this one, the _inout, and the _out below
can be marked static - they are not need outside of sock.c.


> +{
> +	int karg;
> +
> +	if (get_user(karg, (u32 __user *)arg))
> +		return -EFAULT;
> +
> +	return sk->sk_prot->ioctl(sk, cmd, &karg);
> +}
> +
> +/* Copy 'size' bytes from userspace and return `size` back to userspace */
> +int sock_skproto_ioctl_inout(struct sock *sk, unsigned int cmd,
> +			     void __user *arg, size_t size)
> +{
> +	void *ptr;
> +	int ret;
> +
> +	ptr = kmalloc(size, GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(ptr, arg, size)) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	ret = sk->sk_prot->ioctl(sk, cmd, ptr);
> +	if (ret)
> +		goto out;
> +
> +	if (copy_to_user(arg, ptr, size))
> +		ret = -EFAULT;
> +
> +out:
> +	kfree(ptr);
> +	return ret;
> +}
> +
> +/* This is the most common ioctl prep function, where the result (4 bytes) is
> + * copied back to userspace if the ioctl() returns successfully. No input is
> + * copied from userspace as input argument.
> + */
> +int sock_skproto_ioctl_out(struct sock *sk, unsigned int cmd,
> +			   void __user *arg)
> +{
> +	int ret, karg = 0;
> +
> +	ret = sk->sk_prot->ioctl(sk, cmd, &karg);
> +	if (ret)
> +		return ret;
> +
> +	return put_user(karg, (int __user *)arg);
> +}
> +
> +/* A wrapper around sock ioctls, which copies the data from userspace
> + * (depending on the protocol/ioctl), and copies back the result to userspace.
> + * The main motivation for this function is to pass kernel memory to the
> + * protocol ioctl callsback, instead of userspace memory.
> + */
> +int sock_skprot_ioctl(struct sock *sk, unsigned int cmd,
> +		      void __user *arg)
> +{
> +#ifdef CONFIG_IP_MROUTE
> +	if (!strcmp(sk->sk_prot->name, "RAW")) {
> +		switch (cmd) {
> +		/* These userspace buffers will be consumed by ipmr_ioctl() */
> +		case SIOCGETVIFCNT:
> +			return sock_skproto_ioctl_inout(sk, cmd,
> +				arg, sizeof(struct sioc_vif_req));
> +		case SIOCGETSGCNT:
> +			return sock_skproto_ioctl_inout(sk, cmd,
> +				arg, sizeof(struct sioc_sg_req));
> +		}
> +	}
> +#endif
> +#ifdef CONFIG_IPV6_MROUTE
> +	if (!strcmp(sk->sk_prot->name, "RAW6")) {
> +		switch (cmd) {
> +		/* These userspace buffers will be consumed by ip6mr_ioctl() */
> +		case SIOCGETMIFCNT_IN6:
> +			return sock_skproto_ioctl_inout(sk, cmd,
> +				arg, sizeof(struct sioc_mif_req6));
> +		case SIOCGETSGCNT_IN6:
> +			return sock_skproto_ioctl_inout(sk, cmd,
> +				arg, sizeof(struct sioc_sg_req6));
> +		}
> +	}
> +#endif
> +#ifdef CONFIG_PHONET
> +	if (!strcmp(sk->sk_prot->name, "PHONET")) {
> +		/* This userspace buffers will be consumed by pn_ioctl() */
> +		switch (cmd) {
> +		case SIOCPNADDRESOURCE:
> +		case SIOCPNDELRESOURCE:
> +			return sock_skproto_ioctl_in(sk, cmd, arg);
> +		}
> +	}
> +#endif
> +
> +	return sock_skproto_ioctl_out(sk, cmd, arg);
> +}