Recently, we got two syzkaller problems because of oversize packet
when napi frags enabled.

One of the problems is because the first seg size of the iov_iter
from user space is very big, it is 2147479538 which is bigger than
the threshold value for bail out early in __alloc_pages(). And
skb->pfmemalloc is true, __kmalloc_reserve() would use pfmemalloc
reserves without __GFP_NOWARN flag. Thus we got a warning as following:

========================================================
WARNING: CPU: 1 PID: 17965 at mm/page_alloc.c:5295 __alloc_pages+0x1308/0x16c4 mm/page_alloc.c:5295
...
Call trace:
 __alloc_pages+0x1308/0x16c4 mm/page_alloc.c:5295
 __alloc_pages_node include/linux/gfp.h:550 [inline]
 alloc_pages_node include/linux/gfp.h:564 [inline]
 kmalloc_large_node+0x94/0x350 mm/slub.c:4038
 __kmalloc_node_track_caller+0x620/0x8e4 mm/slub.c:4545
 __kmalloc_reserve.constprop.0+0x1e4/0x2b0 net/core/skbuff.c:151
 pskb_expand_head+0x130/0x8b0 net/core/skbuff.c:1654
 __skb_grow include/linux/skbuff.h:2779 [inline]
 tun_napi_alloc_frags+0x144/0x610 drivers/net/tun.c:1477
 tun_get_user+0x31c/0x2010 drivers/net/tun.c:1835
 tun_chr_write_iter+0x98/0x100 drivers/net/tun.c:2036

The other problem is because odd IPv6 packets without NEXTHDR_NONE
extension header and have big packet length, it is 2127925 which is
bigger than ETH_MAX_MTU(65535). After ipv6_gso_pull_exthdrs() in
ipv6_gro_receive(), network_header offset and transport_header offset
are all bigger than U16_MAX. That would trigger skb->network_header
and skb->transport_header overflow error, because they are all '__u16'
type. Eventually, it would affect the value for __skb_push(skb, value),
and make it be a big value. After __skb_push() in ipv6_gro_receive(),
skb->data would less than skb->head, an out of bounds memory bug occurred.
That would trigger the problem as following:

==================================================================
BUG: KASAN: use-after-free in eth_type_trans+0x100/0x260
...
Call trace:
 dump_backtrace+0xd8/0x130
 show_stack+0x1c/0x50
 dump_stack_lvl+0x64/0x7c
 print_address_description.constprop.0+0xbc/0x2e8
 print_report+0x100/0x1e4
 kasan_report+0x80/0x120
 __asan_load8+0x78/0xa0
 eth_type_trans+0x100/0x260
 napi_gro_frags+0x164/0x550
 tun_get_user+0xda4/0x1270
 tun_chr_write_iter+0x74/0x130
 do_iter_readv_writev+0x130/0x1ec
 do_iter_write+0xbc/0x1e0
 vfs_writev+0x13c/0x26c

Restrict the packet size less than ETH_MAX_MTU to fix the problems.
Add len check in tun_napi_alloc_frags() simply. Athough that makes
some kinds of packets payload size slightly smaller than the length
allowed by the protocol, for example, ETH_HLEN + sizeof(struct ipv6hdr)
smaller when the tun device type is IFF_TAP and the packet is IPv6. But
I think that the effect is small and can be ignored.

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 27c6d235cbda..98d3160fcae2 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1459,7 +1459,7 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
 	int err;
 	int i;
 
-	if (it->nr_segs > MAX_SKB_FRAGS + 1)
+	if (it->nr_segs > MAX_SKB_FRAGS + 1 || len > ETH_MAX_MTU)
 		return ERR_PTR(-EMSGSIZE);
 
 	local_bh_disable();
-- 
2.25.1

On Fri, Oct 28, 2022 at 8:32 PM Ziyang Xuan
<william.xuanziyang@huawei.com> wrote:
>
> Recently, we got two syzkaller problems because of oversize packet
> when napi frags enabled.
>
> One of the problems is because the first seg size of the iov_iter
> from user space is very big, it is 2147479538 which is bigger than
> the threshold value for bail out early in __alloc_pages(). And
> skb->pfmemalloc is true, __kmalloc_reserve() would use pfmemalloc
> reserves without __GFP_NOWARN flag. Thus we got a warning as following:
>
> ========================================================
>

> Restrict the packet size less than ETH_MAX_MTU to fix the problems.
> Add len check in tun_napi_alloc_frags() simply. Athough that makes
> some kinds of packets payload size slightly smaller than the length
> allowed by the protocol, for example, ETH_HLEN + sizeof(struct ipv6hdr)
> smaller when the tun device type is IFF_TAP and the packet is IPv6. But
> I think that the effect is small and can be ignored.

I am not sure about ETH_MAX_MTU being completely safe.

napi_get_frags() / napi_alloc_skb() is reserving NET_SKB_PAD +
NET_IP_ALIGN bytes.

transport_header being an offset from skb->head,
we probably want to use (ETH_MAX_MTU - NET_SKB_PAD - NET_IP_ALIGN)

My objection to your initial patch was that you were using PAGE_SIZE,
while Ethernet MTU can easily be ~9000

But 0xFFFF is a bit too much/risky.

Thanks.

>
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
> ---
>  drivers/net/tun.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 27c6d235cbda..98d3160fcae2 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1459,7 +1459,7 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
>         int err;
>         int i;
>
> -       if (it->nr_segs > MAX_SKB_FRAGS + 1)
> +       if (it->nr_segs > MAX_SKB_FRAGS + 1 || len > ETH_MAX_MTU)
>                 return ERR_PTR(-EMSGSIZE);
>
>         local_bh_disable();
> --
> 2.25.1
>

> On Fri, Oct 28, 2022 at 8:32 PM Ziyang Xuan
> <william.xuanziyang@huawei.com> wrote:
>>
>> Recently, we got two syzkaller problems because of oversize packet
>> when napi frags enabled.
>>
>> One of the problems is because the first seg size of the iov_iter
>> from user space is very big, it is 2147479538 which is bigger than
>> the threshold value for bail out early in __alloc_pages(). And
>> skb->pfmemalloc is true, __kmalloc_reserve() would use pfmemalloc
>> reserves without __GFP_NOWARN flag. Thus we got a warning as following:
>>
>> ========================================================
>>
> 
>> Restrict the packet size less than ETH_MAX_MTU to fix the problems.
>> Add len check in tun_napi_alloc_frags() simply. Athough that makes
>> some kinds of packets payload size slightly smaller than the length
>> allowed by the protocol, for example, ETH_HLEN + sizeof(struct ipv6hdr)
>> smaller when the tun device type is IFF_TAP and the packet is IPv6. But
>> I think that the effect is small and can be ignored.
> 
> I am not sure about ETH_MAX_MTU being completely safe.
> 
> napi_get_frags() / napi_alloc_skb() is reserving NET_SKB_PAD +
> NET_IP_ALIGN bytes.
> 
> transport_header being an offset from skb->head,
> we probably want to use (ETH_MAX_MTU - NET_SKB_PAD - NET_IP_ALIGN)

Hi Eric,

Thank you for your review. I did not notice the reserved skb space.
I will fix it in v2 patch.

Thanks.
> 
> My objection to your initial patch was that you were using PAGE_SIZE,
> while Ethernet MTU can easily be ~9000
> 
> But 0xFFFF is a bit too much/risky.
> 
> Thanks.
> 
>>
>> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
>> ---
>>  drivers/net/tun.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 27c6d235cbda..98d3160fcae2 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1459,7 +1459,7 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
>>         int err;
>>         int i;
>>
>> -       if (it->nr_segs > MAX_SKB_FRAGS + 1)
>> +       if (it->nr_segs > MAX_SKB_FRAGS + 1 || len > ETH_MAX_MTU)
>>                 return ERR_PTR(-EMSGSIZE);
>>
>>         local_bh_disable();
>> --
>> 2.25.1
>>
> .
>