| From: Sahitya Tummala <stummala@codeaurora.org> |
| Date: Thu, 3 Jan 2019 16:48:15 +0530 |
| Subject: configfs: Fix use-after-free when accessing sd->s_dentry |
| |
| commit f6122ed2a4f9c9c1c073ddf6308d1b2ac10e0781 upstream. |
| |
| In the vfs_statx() context, during path lookup, the dentry gets |
| added to sd->s_dentry via configfs_attach_attr(). In the end, |
| vfs_statx() kills the dentry by calling path_put(), which invokes |
| configfs_d_iput(). Ideally, this dentry must be removed from |
| sd->s_dentry but it doesn't if the sd->s_count >= 3. As a result, |
| sd->s_dentry is holding reference to a stale dentry pointer whose |
| memory is already freed up. This results in use-after-free issue, |
| when this stale sd->s_dentry is accessed later in |
| configfs_readdir() path. |
| |
| This issue can be easily reproduced, by running the LTP test case - |
| sh fs_racer_file_list.sh /config |
| (https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/fs/racer/fs_racer_file_list.sh) |
| |
| Fixes: 76ae281f6307 ('configfs: fix race between dentry put and lookup') |
| Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> |
| Signed-off-by: Christoph Hellwig <hch@lst.de> |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| fs/configfs/dir.c | 14 ++++++-------- |
| 1 file changed, 6 insertions(+), 8 deletions(-) |
| |
| --- a/fs/configfs/dir.c |
| +++ b/fs/configfs/dir.c |
| @@ -58,15 +58,13 @@ static void configfs_d_iput(struct dentr |
| if (sd) { |
| /* Coordinate with configfs_readdir */ |
| spin_lock(&configfs_dirent_lock); |
| - /* Coordinate with configfs_attach_attr where will increase |
| - * sd->s_count and update sd->s_dentry to new allocated one. |
| - * Only set sd->dentry to null when this dentry is the only |
| - * sd owner. |
| - * If not do so, configfs_d_iput may run just after |
| - * configfs_attach_attr and set sd->s_dentry to null |
| - * even it's still in use. |
| + /* |
| + * Set sd->s_dentry to null only when this dentry is the one |
| + * that is going to be killed. Otherwise configfs_d_iput may |
| + * run just after configfs_attach_attr and set sd->s_dentry to |
| + * NULL even it's still in use. |
| */ |
| - if (atomic_read(&sd->s_count) <= 2) |
| + if (sd->s_dentry == dentry) |
| sd->s_dentry = NULL; |
| |
| spin_unlock(&configfs_dirent_lock); |