]>
Commit | Line | Data |
---|---|---|
a15a8890 SL |
1 | From cedabc41f8db72fa7c5fdb250f0f31c39f2f700b Mon Sep 17 00:00:00 2001 |
2 | From: Sahitya Tummala <stummala@codeaurora.org> | |
3 | Date: Thu, 3 Jan 2019 16:48:15 +0530 | |
4 | Subject: configfs: Fix use-after-free when accessing sd->s_dentry | |
5 | ||
6 | [ Upstream commit f6122ed2a4f9c9c1c073ddf6308d1b2ac10e0781 ] | |
7 | ||
8 | In the vfs_statx() context, during path lookup, the dentry gets | |
9 | added to sd->s_dentry via configfs_attach_attr(). In the end, | |
10 | vfs_statx() kills the dentry by calling path_put(), which invokes | |
11 | configfs_d_iput(). Ideally, this dentry must be removed from | |
12 | sd->s_dentry but it doesn't if the sd->s_count >= 3. As a result, | |
13 | sd->s_dentry is holding reference to a stale dentry pointer whose | |
14 | memory is already freed up. This results in use-after-free issue, | |
15 | when this stale sd->s_dentry is accessed later in | |
16 | configfs_readdir() path. | |
17 | ||
18 | This issue can be easily reproduced, by running the LTP test case - | |
19 | sh fs_racer_file_list.sh /config | |
20 | (https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/fs/racer/fs_racer_file_list.sh) | |
21 | ||
22 | Fixes: 76ae281f6307 ('configfs: fix race between dentry put and lookup') | |
23 | Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> | |
24 | Signed-off-by: Christoph Hellwig <hch@lst.de> | |
25 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
26 | --- | |
27 | fs/configfs/dir.c | 14 ++++++-------- | |
28 | 1 file changed, 6 insertions(+), 8 deletions(-) | |
29 | ||
30 | diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c | |
31 | index 920d350df37b..809c1edffbaf 100644 | |
32 | --- a/fs/configfs/dir.c | |
33 | +++ b/fs/configfs/dir.c | |
34 | @@ -58,15 +58,13 @@ static void configfs_d_iput(struct dentry * dentry, | |
35 | if (sd) { | |
36 | /* Coordinate with configfs_readdir */ | |
37 | spin_lock(&configfs_dirent_lock); | |
38 | - /* Coordinate with configfs_attach_attr where will increase | |
39 | - * sd->s_count and update sd->s_dentry to new allocated one. | |
40 | - * Only set sd->dentry to null when this dentry is the only | |
41 | - * sd owner. | |
42 | - * If not do so, configfs_d_iput may run just after | |
43 | - * configfs_attach_attr and set sd->s_dentry to null | |
44 | - * even it's still in use. | |
45 | + /* | |
46 | + * Set sd->s_dentry to null only when this dentry is the one | |
47 | + * that is going to be killed. Otherwise configfs_d_iput may | |
48 | + * run just after configfs_attach_attr and set sd->s_dentry to | |
49 | + * NULL even it's still in use. | |
50 | */ | |
51 | - if (atomic_read(&sd->s_count) <= 2) | |
52 | + if (sd->s_dentry == dentry) | |
53 | sd->s_dentry = NULL; | |
54 | ||
55 | spin_unlock(&configfs_dirent_lock); | |
56 | -- | |
57 | 2.20.1 | |
58 |