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
| ||
|
Message-Id: <20181005212957.8008-1-tytso@mit.edu> Date: Fri, 5 Oct 2018 17:29:57 -0400 From: Theodore Ts'o <tytso@....edu> To: Ext4 Developers List <linux-ext4@...r.kernel.org> Cc: Theodore Ts'o <tytso@....edu>, stable@...nel.org Subject: [PATCH -v2] ext4: fix use-after-free race in ext4_remount()'s error path It's possible for ext4_show_quota_options() to try reading s_qf_names[i] while it is being modified by ext4_remount() --- most notably, in ext4_remount's error path when the original values of the quota file name gets restored. Reported-by: syzbot+a2872d6feea6918008a9@...kaller.appspotmail.com Signed-off-by: Theodore Ts'o <tytso@....edu> Cc: stable@...nel.org --- The -v1 patch had circular locking problems as reported by Lockdep. So I've redone this change using RCU. fs/ext4/super.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index faf293ed8060..0843fd5ce449 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1578,6 +1578,7 @@ static int clear_qf_name(struct super_block *sb, int qtype) { struct ext4_sb_info *sbi = EXT4_SB(sb); + char *to_free; if (sb_any_quota_loaded(sb) && sbi->s_qf_names[qtype]) { @@ -1585,8 +1586,11 @@ static int clear_qf_name(struct super_block *sb, int qtype) " when quota turned on"); return -1; } - kfree(sbi->s_qf_names[qtype]); - sbi->s_qf_names[qtype] = NULL; + to_free = rcu_dereference_protected(sbi->s_qf_names[qtype], + lockdep_is_held(&sb->s_umount)); + rcu_assign_pointer(sbi->s_qf_names[qtype], NULL); + synchronize_rcu(); + kfree(to_free); return 1; } #endif @@ -2048,11 +2052,15 @@ static inline void ext4_show_quota_options(struct seq_file *seq, seq_printf(seq, ",jqfmt=%s", fmtname); } + rcu_read_lock(); if (sbi->s_qf_names[USRQUOTA]) - seq_show_option(seq, "usrjquota", sbi->s_qf_names[USRQUOTA]); + seq_show_option(seq, "usrjquota", + rcu_dereference(sbi->s_qf_names[USRQUOTA])); if (sbi->s_qf_names[GRPQUOTA]) - seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]); + seq_show_option(seq, "grpjquota", + rcu_dereference(sbi->s_qf_names[GRPQUOTA])); + rcu_read_unlock(); #endif } @@ -5104,6 +5112,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) int err = 0; #ifdef CONFIG_QUOTA int i, j; + char *to_free[EXT4_MAXQUOTAS]; #endif char *orig_data = kstrdup(data, GFP_KERNEL); @@ -5353,9 +5362,13 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) #ifdef CONFIG_QUOTA sbi->s_jquota_fmt = old_opts.s_jquota_fmt; for (i = 0; i < EXT4_MAXQUOTAS; i++) { - kfree(sbi->s_qf_names[i]); - sbi->s_qf_names[i] = old_opts.s_qf_names[i]; + to_free[i] = rcu_dereference_protected(sbi->s_qf_names[i], + &sb->s_umount); + rcu_assign_pointer(sbi->s_qf_names[i], old_opts.s_qf_names[i]); } + for (i = 0; i < EXT4_MAXQUOTAS; i++) + kfree(to_free[i]); + synchronize_rcu(); #endif kfree(orig_data); return err; -- 2.18.0.rc0
Powered by blists - more mailing lists