Hi,
On Mon, Mar 11, 2019 at 08:17 PM CET, Sasha Levin wrote:
This is a note to let you know that I've just added the patch titled
sk_msg: Always cancel strp work before freeing the psockto the 4.20-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: sk_msg-always-cancel-strp-work-before-freeing-the-ps.patch and it can be found in the queue-4.20 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
There is a follow-up fix for this change - e8e3437762ad ("bpf: Stop the psock parser before canceling its work") in bpf tree:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=e8e34...
I think this patch should not go to into stable without the follow-up fix. Otherwise kselftests for bpf will be noisy due to kernel warnings.
Thanks, Jakub
commit d3747ebdfd2fd5e12d8d04ad3f6261503ce52913 Author: Jakub Sitnicki jakub@cloudflare.com Date: Mon Jan 28 10:13:35 2019 +0100
sk_msg: Always cancel strp work before freeing the psock [ Upstream commit 1d79895aef18fa05789995d86d523c9b2ee58a02 ] Despite having stopped the parser, we still need to deinitialize it by calling strp_done so that it cancels its work. Otherwise the worker thread can run after we have freed the parser, and attempt to access its workqueue resulting in a use-after-free: ================================================================== BUG: KASAN: use-after-free in pwq_activate_delayed_work+0x1b/0x1d0 Read of size 8 at addr ffff888069975240 by task kworker/u2:2/93 CPU: 0 PID: 93 Comm: kworker/u2:2 Not tainted 5.0.0-rc2-00335-g28f9d1a3d4fe-dirty #14 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-2.fc27 04/01/2014 Workqueue: (null) (kstrp) Call Trace: print_address_description+0x6e/0x2b0 ? pwq_activate_delayed_work+0x1b/0x1d0 kasan_report+0xfd/0x177 ? pwq_activate_delayed_work+0x1b/0x1d0 ? pwq_activate_delayed_work+0x1b/0x1d0 pwq_activate_delayed_work+0x1b/0x1d0 ? process_one_work+0x4aa/0x660 pwq_dec_nr_in_flight+0x9b/0x100 worker_thread+0x82/0x680 ? process_one_work+0x660/0x660 kthread+0x1b9/0x1e0 ? __kthread_create_on_node+0x250/0x250 ret_from_fork+0x1f/0x30 Allocated by task 111: sk_psock_init+0x3c/0x1b0 sock_map_link.isra.2+0x103/0x4b0 sock_map_update_common+0x94/0x270 sock_map_update_elem+0x145/0x160 __se_sys_bpf+0x152e/0x1e10 do_syscall_64+0xb2/0x3e0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Freed by task 112: kfree+0x7f/0x140 process_one_work+0x40b/0x660 worker_thread+0x82/0x680 kthread+0x1b9/0x1e0 ret_from_fork+0x1f/0x30 The buggy address belongs to the object at ffff888069975180 which belongs to the cache kmalloc-512 of size 512 The buggy address is located 192 bytes inside of 512-byte region [ffff888069975180, ffff888069975380) The buggy address belongs to the page: page:ffffea0001a65d00 count:1 mapcount:0 mapping:ffff88806d401280 index:0x0 compound_mapcount: 0 flags: 0x4000000000010200(slab|head) raw: 4000000000010200 dead000000000100 dead000000000200 ffff88806d401280 raw: 0000000000000000 00000000800c000c 00000001ffffffff 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff888069975100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff888069975180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb >ffff888069975200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ^ ffff888069975280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff888069975300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ================================================================== Reported-by: Marek Majkowski <marek@cloudflare.com> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> Link: https://lore.kernel.org/netdev/CAJPywTLwgXNEZ2dZVoa=udiZmtrWJ0q5SuBW64aYs0Y1khXX3A@mail.gmail.com Acked-by: Song Liu <songliubraving@fb.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Sasha Levin <sashal@kernel.org>diff --git a/net/core/skmsg.c b/net/core/skmsg.c index 54d854807630..4932861d7b88 100644 --- a/net/core/skmsg.c +++ b/net/core/skmsg.c @@ -545,8 +545,7 @@ static void sk_psock_destroy_deferred(struct work_struct *gc) struct sk_psock *psock = container_of(gc, struct sk_psock, gc);
/* No sk_callback_lock since already detached. */
- if (psock->parser.enabled)
strp_done(&psock->parser.strp);
strp_done(&psock->parser.strp);
cancel_work_sync(&psock->work);