Messages in this thread | | | From | Willem de Bruijn <> | Date | Thu, 6 Aug 2020 14:25:54 +0200 | Subject | Re: [PATCH 1/5] net: Fix potential deadloop in skb_copy_ubufs() |
| |
On Thu, Aug 6, 2020 at 1:48 PM linmiaohe <linmiaohe@huawei.com> wrote: > > From: Miaohe Lin <linmiaohe@huawei.com> > > We could be trapped in deadloop when we try to copy userspace skb frags > buffers to kernel with a cloned skb: > > [kbox] catch panic event, panic reason:kernel stack overflow > [kbox] catch panic event, start logging. > CPU: 3 PID: 4083 Comm: insmod Kdump: loaded Tainted: G OE 4.19 #6 > Hardware name: linux,dummy-virt (DT) > Call trace: > dump_backtrace+0x0/0x198 > show_stack+0x24/0x30 > dump_stack+0xa4/0xcc > kbox_panic_notifier_callback+0x1d0/0x310 [kbox] > notifier_call_chain+0x5c/0xa0 > atomic_notifier_call_chain+0x3c/0x50 > panic+0x164/0x314 > __stack_chk_fail+0x0/0x28 > handle_bad_stack+0xfc/0x108 > __bad_stack+0x90/0x94 > pskb_expand_head+0x0/0x2c8 > pskb_expand_head+0x290/0x2c8 > skb_copy_ubufs+0x3cc/0x520 > pskb_expand_head+0x290/0x2c8 > skb_copy_ubufs+0x3cc/0x520 > pskb_expand_head+0x290/0x2c8 > skb_copy_ubufs+0x3cc/0x520 > pskb_expand_head+0x290/0x2c8 > skb_copy_ubufs+0x3cc/0x520 > ... > pskb_expand_head+0x290/0x2c8 > skb_copy_ubufs+0x3cc/0x520 > ... > > Reproduce code snippet: > skb = alloc_skb(UBUF_DATA_LEN, GFP_ATOMIC); > clone = skb_clone(skb, GFP_ATOMIC); > skb_zcopy_set_nouarg(clone, NULL); > pskb_expand_head(skb, 0, 0, GFP_ATOMIC); > > Catch this unexpected case and return -EINVAL in skb_orphan_frags() before > we call skb_copy_ubufs() to fix it.
Is this a hypothetical codepath?
skb zerocopy carefully tracks clone calls where necessary. See the call to skb_orphan_frags in skb_clone, and the implementation of that callee.
The only caller of skb zerocopy with nouarg is tpacket_fill_skb, as of commit 5cd8d46ea156 ("packet: copy user buffers before orphan or clone").
As the commit subject indicates, this sets skb_zcopy_set_nouarg exactly to be sure that any clone will trigger a copy of "zerocopy" user data to private kernel memory.
No clone must happen between alloc_skb and skb_zcopy_set_nouarg, indeed. But AFAIK, none exists.
| |