Re: [PATCH net] tun: complete the zerocopy buffer of packets consumed by XDP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] tun: type confusion in tun_napi_alloc_frags() [1]
--

commit 432dd92fb61360561c1a6f28a7f9ecf8e823ad4c
Author: Antonio Quartulli <antonio@mandelbit.com>

tun: complete the zerocopy buffer of packets consumed by XDP

This commit completes the caller's zerocopy ubuf_info as copied when
tun_build_skb() copies the packet and native XDP consumes it.

Link: https://sashiko.dev/#/bug/linux-f9997fb6-303f-41d6-93e2-60c392b1856f

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 86fd44e107550..9271968407253 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1871,8 +1871,15 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		err = PTR_ERR_OR_ZERO(skb);
>  		if (err)
>  			goto drop;
> -		if (!skb)
> +		if (!skb) {
> +			/* consumed by XDP, after copying */
> +			if (msg_control) {
> +				struct ubuf_info *uarg = msg_control;
> +
> +				uarg->ops->complete(NULL, uarg, false);
> +			}
>  			return total_len;
> +		}
>  	} else {
>  		if (!zerocopy) {
>  			copylen = len;

[Severity: Critical]
This problem wasn't introduced by this patch, but can
tun_napi_alloc_frags() in the adjacent else branch corrupt memory when
tun_get_user() is called with a non-iovec iterator [1]?

drivers/net/tun.c:tun_get_user() {
    ...
		if (frags) {
			mutex_lock(&tfile->napi_mutex);
			skb = tun_napi_alloc_frags(tfile, copylen, from);
    ...
}

drivers/net/tun.c:tun_napi_alloc_frags() {
    ...
	for (i = 1; i < it->nr_segs; i++) {
		const struct iovec *iov = iter_iov(it) + i;
		size_t fragsz = iov->iov_len;
    ...
		frag = netdev_alloc_frag(fragsz);
    ...
}

When IFF_NAPI_FRAGS is enabled and a caller writes to the tun device using
an ITER_BVEC iterator (for example via splice() or io_uring),
tun_napi_alloc_frags() treats the bio_vec array as an iovec array via
iter_iov(it). Reading iov->iov_len from a struct bio_vec produces a bogus
fragment size from bv_len and bv_offset, causing skb_copy_datagram_from_iter()
to overflow the allocated skb fragments.

[1] https://sashiko.dev/#/bug/linux-ddceb3c7-6f22-48cf-b390-d7b534490c8f

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113232843.2338734-1-antonio@mandelbit.com?part=1