linux-kernel - [PATCH] epoll: Fix UAF dentry name access in wakeup source setup

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 29 Apr 2020 04:31:04 +0200
From:   Jann Horn <jannh@...gle.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Arve Hjønnevåg" <arve@...roid.com>,
        NeilBrown <neilb@...e.de>, "Rafael J . Wysocki" <rjw@...k.pl>
Subject: [PATCH] epoll: Fix UAF dentry name access in wakeup source setup

In ep_create_wakeup_source(), epi->ffd.file is some random file we're
watching with epoll, so it might well be renamed concurrently. And when a
file gets renamed, the buffer containing its name may be freed.

This can be reproduced by racing a task that keeps adding and removing
EPOLLWAKEUP epoll entries for a fifo with another task that keeps renaming
the fifo between two long names if you add an mdelay(200) call directly
before wakeup_source_register(); KASAN then complains:

BUG: KASAN: use-after-free in strlen+0xa/0x40
Read of size 1 at addr ffff888065fda990 by task wakemeup/2375
[...]
Call Trace:
[...]
 strlen+0xa/0x40
 kstrdup+0x1a/0x60
 wakeup_source_create+0x43/0xb0
 wakeup_source_register+0x13/0x60
 ep_create_wakeup_source+0x7f/0xf0
 do_epoll_ctl+0x13d0/0x1880
[...]
 __x64_sys_epoll_ctl+0xc3/0x110
[...]
Allocated by task 2376:
[...]
 __d_alloc+0x323/0x3c0
 d_alloc+0x30/0xf0
 __lookup_hash+0x61/0xc0
 do_renameat2+0x3fa/0x6d0
 __x64_sys_rename+0x3a/0x40
[...]
Freed by task 2379:
[...]
 kfree_rcu_work+0x9b/0x5d0
[...]

Backporting note: This patch depends on commit 49d31c2f389a ("dentry name
snapshots"). Maybe that one should also be backported as a dependency for
pre-v4.13? (Sorry, I wasn't sure how to properly express this as a "Fixes:"
tag.)

Cc: stable@...r.kernel.org
Fixes: 4d7e30d98939 ("epoll: Add a flag, EPOLLWAKEUP, to prevent suspend while epoll events are ready")
Signed-off-by: Jann Horn <jannh@...gle.com>
---
I'm guessing this will go through akpm's tree?

 fs/eventpoll.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 8c596641a72b0..5052a41670479 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1450,7 +1450,7 @@ static int reverse_path_check(void)
 
 static int ep_create_wakeup_source(struct epitem *epi)
 {
-	const char *name;
+	struct name_snapshot name;
 	struct wakeup_source *ws;
 
 	if (!epi->ep->ws) {
@@ -1459,8 +1459,9 @@ static int ep_create_wakeup_source(struct epitem *epi)
 			return -ENOMEM;
 	}
 
-	name = epi->ffd.file->f_path.dentry->d_name.name;
-	ws = wakeup_source_register(NULL, name);
+	take_dentry_name_snapshot(&name, epi->ffd.file->f_path.dentry);
+	ws = wakeup_source_register(NULL, name.name.name);
+	release_dentry_name_snapshot(&name);
 
 	if (!ws)
 		return -ENOMEM;

base-commit: 96c9a7802af7d500a582d89a8b864584fe878c1b
-- 
2.26.2.303.gf8c07b1a785-goog