linux-ext4 - [PATCH 1/2] ext4: don't use kfree() on rcu protected pointer sbi->s_qf

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]

Message-Id: <20220104143518.134465-1-lczerner@redhat.com>
Date:   Tue,  4 Jan 2022 15:35:17 +0100
From:   Lukas Czerner <lczerner@...hat.com>
To:     linux-ext4@...r.kernel.org, tytso@....edu
Cc:     kernel test robot <lkp@...el.com>
Subject: [PATCH 1/2] ext4: don't use kfree() on rcu protected pointer sbi->s_qf_names

During ext4 mount api rework the commit e6e268cb6822 ("ext4: move quota
configuration out of handle_mount_opt()") introduced a bug where we
would kfree(sbi->s_qf_names[i]) before assigning the new quota name in
ext4_apply_quota_options().

This is wrong because we're using kfree() on rcu prointer that could be
simultaneously accessed from ext4_show_quota_options() during remount.
Fix it by using rcu_replace_pointer() to replace the old qname with the
new one and then kfree_rcu() the old quota name.

Also use get_qf_name() instead of sbi->s_qf_names in strcmp() to silence
the sparse warning.

Fixes: e6e268cb6822 ("ext4: move quota configuration out of handle_mount_opt()")
Reported-by: kernel test robot <lkp@...el.com>
Signed-off-by: Lukas Czerner <lczerner@...hat.com>
---
 fs/ext4/super.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b72d989b77fb..acb0c58cd3d1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2633,8 +2633,10 @@ static void ext4_apply_quota_options(struct fs_context *fc,
 
 			qname = ctx->s_qf_names[i]; /* May be NULL */
 			ctx->s_qf_names[i] = NULL;
-			kfree(sbi->s_qf_names[i]);
-			rcu_assign_pointer(sbi->s_qf_names[i], qname);
+			qname = rcu_replace_pointer(sbi->s_qf_names[i], qname,
+						lockdep_is_held(&sb->s_umount));
+			if (qname)
+				kfree_rcu(qname);
 			set_opt(sb, QUOTA);
 		}
 	}
@@ -2688,7 +2690,7 @@ static int ext4_check_quota_consistency(struct fs_context *fc,
 				goto err_jquota_change;
 
 			if (sbi->s_qf_names[i] && ctx->s_qf_names[i] &&
-			    strcmp(sbi->s_qf_names[i],
+			    strcmp(get_qf_name(sb, sbi, i),
 				   ctx->s_qf_names[i]) != 0)
 				goto err_jquota_specified;
 		}
-- 
2.31.1