netdev - Re: [PATCH v3] drivers: net: prevent tun_get

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 27 Jul 2023 18:11:57 -0600
From: David Ahern <dsahern@...il.com>
To: Andrew Kanner <andrew.kanner@...il.com>,
 Jesper Dangaard Brouer <jbrouer@...hat.com>
Cc: Paolo Abeni <pabeni@...hat.com>, Jason Wang <jasowang@...hat.com>,
 brouer@...hat.com, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-kernel-mentees@...ts.linuxfoundation.org,
 syzbot+f817490f5bd20541b90a@...kaller.appspotmail.com,
 John Fastabend <john.fastabend@...il.com>
Subject: Re: [PATCH v3] drivers: net: prevent tun_get_user() to exceed xdp
 size limits

On 7/27/23 5:48 PM, Andrew Kanner wrote:
> 
> Thanks, everyone.
> 
> If we summarize the discussion - there are 3 issues here:
> 1. tun_can_build_skb() doesn't count XDP_PACKET_HEADROOM (minor and
>    most trivial)
> 2. WARN_ON_ONCE from net/core/filter.c, which may be too strict / not
>    needed at all.
> 3. strange behaviour with reallocationg SKB (65007 -> 131072)

I believe that happens because of the current skb size and the need to
expand it to account for the XDP headroom makes the allocation go over
64kB. Since tun is given the packet via a write call there are no header
markers to allocate separate space for headers and data (e.g. like TCP
does with 32kB data segments).

> 
> I can check these issues. I have to dive a little deeper with 2-3,
> most likely with kgdb and syzkaller repro. But seems this is not
> somewhat urgent and lives quite a long time without being noticed.
> 
> BTW: Attached the ftrace logs using the original syzkaller repro
> (starting with tun_get_user()). They answer Jesper's question about
> contiguous physical memory allocation (kmem_cache_alloc_node() /
> kmalloc_reserve()). But I'll check it one more time before submitting
> a new PATCH V4 or another patch / patch series.
>