tmpfs: fix race on handling dquot rbtree
authorCarlos Maiolino <cem@kernel.org>
Wed, 20 Mar 2024 12:39:59 +0000 (13:39 +0100)
committerAndrew Morton <akpm@linux-foundation.org>
Tue, 26 Mar 2024 18:07:23 +0000 (11:07 -0700)
A syzkaller reproducer found a race while attempting to remove dquot
information from the rb tree.

Fetching the rb_tree root node must also be protected by the
dqopt->dqio_sem, otherwise, giving the right timing, shmem_release_dquot()
will trigger a warning because it couldn't find a node in the tree, when
the real reason was the root node changing before the search starts:

Thread 1 Thread 2
- shmem_release_dquot() - shmem_{acquire,release}_dquot()

- fetch ROOT - Fetch ROOT

- acquire dqio_sem
- wait dqio_sem

- do something, triger a tree rebalance
- release dqio_sem

- acquire dqio_sem
- start searching for the node, but
  from the wrong location, missing
  the node, and triggering a warning.

Link: https://lkml.kernel.org/r/20240320124011.398847-1-cem@kernel.org
Fixes: eafc474e2029 ("shmem: prepare shmem quota infrastructure")
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Reported-by: Ubisectech Sirius <bugreport@ubisectech.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: Hugh Dickins <hughd@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
mm/shmem_quota.c

index 062d1c1097ae35fdb9fc87faa722bb403c1804fd..ce514e700d2f65f4b23ae9d579b78790505cc540 100644 (file)
@@ -116,7 +116,7 @@ static int shmem_free_file_info(struct super_block *sb, int type)
 static int shmem_get_next_id(struct super_block *sb, struct kqid *qid)
 {
        struct mem_dqinfo *info = sb_dqinfo(sb, qid->type);
-       struct rb_node *node = ((struct rb_root *)info->dqi_priv)->rb_node;
+       struct rb_node *node;
        qid_t id = from_kqid(&init_user_ns, *qid);
        struct quota_info *dqopt = sb_dqopt(sb);
        struct quota_id *entry = NULL;
@@ -126,6 +126,7 @@ static int shmem_get_next_id(struct super_block *sb, struct kqid *qid)
                return -ESRCH;
 
        down_read(&dqopt->dqio_sem);
+       node = ((struct rb_root *)info->dqi_priv)->rb_node;
        while (node) {
                entry = rb_entry(node, struct quota_id, node);
 
@@ -165,7 +166,7 @@ out_unlock:
 static int shmem_acquire_dquot(struct dquot *dquot)
 {
        struct mem_dqinfo *info = sb_dqinfo(dquot->dq_sb, dquot->dq_id.type);
-       struct rb_node **n = &((struct rb_root *)info->dqi_priv)->rb_node;
+       struct rb_node **n;
        struct shmem_sb_info *sbinfo = dquot->dq_sb->s_fs_info;
        struct rb_node *parent = NULL, *new_node = NULL;
        struct quota_id *new_entry, *entry;
@@ -176,6 +177,8 @@ static int shmem_acquire_dquot(struct dquot *dquot)
        mutex_lock(&dquot->dq_lock);
 
        down_write(&dqopt->dqio_sem);
+       n = &((struct rb_root *)info->dqi_priv)->rb_node;
+
        while (*n) {
                parent = *n;
                entry = rb_entry(parent, struct quota_id, node);
@@ -264,7 +267,7 @@ static bool shmem_is_empty_dquot(struct dquot *dquot)
 static int shmem_release_dquot(struct dquot *dquot)
 {
        struct mem_dqinfo *info = sb_dqinfo(dquot->dq_sb, dquot->dq_id.type);
-       struct rb_node *node = ((struct rb_root *)info->dqi_priv)->rb_node;
+       struct rb_node *node;
        qid_t id = from_kqid(&init_user_ns, dquot->dq_id);
        struct quota_info *dqopt = sb_dqopt(dquot->dq_sb);
        struct quota_id *entry = NULL;
@@ -275,6 +278,7 @@ static int shmem_release_dquot(struct dquot *dquot)
                goto out_dqlock;
 
        down_write(&dqopt->dqio_sem);
+       node = ((struct rb_root *)info->dqi_priv)->rb_node;
        while (node) {
                entry = rb_entry(node, struct quota_id, node);