Skip to content

Commit 33458ea

Browse files
tytsotytso
committed
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@syzkaller.appspotmail.com Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@kernel.org # 3.2+
1 parent 6fd9417 commit 33458ea

File tree

2 files changed

+50
-26
lines changed

2 files changed

+50
-26
lines changed

‎fs/ext4/ext4.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1405,7 +1405,8 @@ struct ext4_sb_info {
14051405
u32 s_min_batch_time;
14061406
struct block_device *journal_bdev;
14071407
#ifdef CONFIG_QUOTA
1408-
char *s_qf_names[EXT4_MAXQUOTAS]; /* Names of quota files with journalled quota */
1408+
/* Names of quota files with journalled quota */
1409+
char __rcu *s_qf_names[EXT4_MAXQUOTAS];
14091410
int s_jquota_fmt; /* Format of quota to use */
14101411
#endif
14111412
unsigned int s_want_extra_isize; /* New inodes should reserve # bytes */

‎fs/ext4/super.c

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,18 @@ static inline void ext4_quota_off_umount(struct super_block *sb)
914914
for (type = 0; type < EXT4_MAXQUOTAS; type++)
915915
ext4_quota_off(sb, type);
916916
}
917+
918+
/*
919+
* This is a helper function which is used in the mount/remount
920+
* codepaths (which holds s_umount) to fetch the quota file name.
921+
*/
922+
static inline char *get_qf_name(struct super_block *sb,
923+
struct ext4_sb_info *sbi,
924+
int type)
925+
{
926+
return rcu_dereference_protected(sbi->s_qf_names[type],
927+
lockdep_is_held(&sb->s_umount));
928+
}
917929
#else
918930
static inline void ext4_quota_off_umount(struct super_block *sb)
919931
{
@@ -965,7 +977,7 @@ static void ext4_put_super(struct super_block *sb)
965977
percpu_free_rwsem(&sbi->s_journal_flag_rwsem);
966978
#ifdef CONFIG_QUOTA
967979
for (i = 0; i < EXT4_MAXQUOTAS; i++)
968-
kfree(sbi->s_qf_names[i]);
980+
kfree(get_qf_name(sb, sbi, i));
969981
#endif
970982

971983
/* Debugging code just in case the in-memory inode orphan list
@@ -1531,11 +1543,10 @@ static const char deprecated_msg[] =
15311543
static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
15321544
{
15331545
struct ext4_sb_info *sbi = EXT4_SB(sb);
1534-
char *qname;
1546+
char *qname, *old_qname = get_qf_name(sb, sbi, qtype);
15351547
int ret = -1;
15361548

1537-
if (sb_any_quota_loaded(sb) &&
1538-
!sbi->s_qf_names[qtype]) {
1549+
if (sb_any_quota_loaded(sb) && !old_qname) {
15391550
ext4_msg(sb, KERN_ERR,
15401551
"Cannot change journaled "
15411552
"quota options when quota turned on");
@@ -1552,8 +1563,8 @@ static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
15521563
"Not enough memory for storing quotafile name");
15531564
return -1;
15541565
}
1555-
if (sbi->s_qf_names[qtype]) {
1556-
if (strcmp(sbi->s_qf_names[qtype], qname) == 0)
1566+
if (old_qname) {
1567+
if (strcmp(old_qname, qname) == 0)
15571568
ret = 1;
15581569
else
15591570
ext4_msg(sb, KERN_ERR,
@@ -1566,7 +1577,7 @@ static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
15661577
"quotafile must be on filesystem root");
15671578
goto errout;
15681579
}
1569-
sbi->s_qf_names[qtype] = qname;
1580+
rcu_assign_pointer(sbi->s_qf_names[qtype], qname);
15701581
set_opt(sb, QUOTA);
15711582
return 1;
15721583
errout:
@@ -1578,15 +1589,16 @@ static int clear_qf_name(struct super_block *sb, int qtype)
15781589
{
15791590

15801591
struct ext4_sb_info *sbi = EXT4_SB(sb);
1592+
char *old_qname = get_qf_name(sb, sbi, qtype);
15811593

1582-
if (sb_any_quota_loaded(sb) &&
1583-
sbi->s_qf_names[qtype]) {
1594+
if (sb_any_quota_loaded(sb) && old_qname) {
15841595
ext4_msg(sb, KERN_ERR, "Cannot change journaled quota options"
15851596
" when quota turned on");
15861597
return -1;
15871598
}
1588-
kfree(sbi->s_qf_names[qtype]);
1589-
sbi->s_qf_names[qtype] = NULL;
1599+
rcu_assign_pointer(sbi->s_qf_names[qtype], NULL);
1600+
synchronize_rcu();
1601+
kfree(old_qname);
15901602
return 1;
15911603
}
15921604
#endif
@@ -1961,7 +1973,7 @@ static int parse_options(char *options, struct super_block *sb,
19611973
int is_remount)
19621974
{
19631975
struct ext4_sb_info *sbi = EXT4_SB(sb);
1964-
char *p;
1976+
char *p, __maybe_unused *usr_qf_name, __maybe_unused *grp_qf_name;
19651977
substring_t args[MAX_OPT_ARGS];
19661978
int token;
19671979

@@ -1992,11 +2004,13 @@ static int parse_options(char *options, struct super_block *sb,
19922004
"Cannot enable project quota enforcement.");
19932005
return 0;
19942006
}
1995-
if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
1996-
if (test_opt(sb, USRQUOTA) && sbi->s_qf_names[USRQUOTA])
2007+
usr_qf_name = get_qf_name(sb, sbi, USRQUOTA);
2008+
grp_qf_name = get_qf_name(sb, sbi, GRPQUOTA);
2009+
if (usr_qf_name || grp_qf_name) {
2010+
if (test_opt(sb, USRQUOTA) && usr_qf_name)
19972011
clear_opt(sb, USRQUOTA);
19982012

1999-
if (test_opt(sb, GRPQUOTA) && sbi->s_qf_names[GRPQUOTA])
2013+
if (test_opt(sb, GRPQUOTA) && grp_qf_name)
20002014
clear_opt(sb, GRPQUOTA);
20012015

20022016
if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA)) {
@@ -2030,6 +2044,7 @@ static inline void ext4_show_quota_options(struct seq_file *seq,
20302044
{
20312045
#if defined(CONFIG_QUOTA)
20322046
struct ext4_sb_info *sbi = EXT4_SB(sb);
2047+
char *usr_qf_name, *grp_qf_name;
20332048

20342049
if (sbi->s_jquota_fmt) {
20352050
char *fmtname = "";
@@ -2048,11 +2063,14 @@ static inline void ext4_show_quota_options(struct seq_file *seq,
20482063
seq_printf(seq, ",jqfmt=%s", fmtname);
20492064
}
20502065

2051-
if (sbi->s_qf_names[USRQUOTA])
2052-
seq_show_option(seq, "usrjquota", sbi->s_qf_names[USRQUOTA]);
2053-
2054-
if (sbi->s_qf_names[GRPQUOTA])
2055-
seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]);
2066+
rcu_read_lock();
2067+
usr_qf_name = rcu_dereference(sbi->s_qf_names[USRQUOTA]);
2068+
grp_qf_name = rcu_dereference(sbi->s_qf_names[GRPQUOTA]);
2069+
if (usr_qf_name)
2070+
seq_show_option(seq, "usrjquota", usr_qf_name);
2071+
if (grp_qf_name)
2072+
seq_show_option(seq, "grpjquota", grp_qf_name);
2073+
rcu_read_unlock();
20562074
#endif
20572075
}
20582076

@@ -5104,6 +5122,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
51045122
int err = 0;
51055123
#ifdef CONFIG_QUOTA
51065124
int i, j;
5125+
char *to_free[EXT4_MAXQUOTAS];
51075126
#endif
51085127
char *orig_data = kstrdup(data, GFP_KERNEL);
51095128

@@ -5123,8 +5142,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
51235142
old_opts.s_jquota_fmt = sbi->s_jquota_fmt;
51245143
for (i = 0; i < EXT4_MAXQUOTAS; i++)
51255144
if (sbi->s_qf_names[i]) {
5126-
old_opts.s_qf_names[i] = kstrdup(sbi->s_qf_names[i],
5127-
GFP_KERNEL);
5145+
char *qf_name = get_qf_name(sb, sbi, i);
5146+
5147+
old_opts.s_qf_names[i] = kstrdup(qf_name, GFP_KERNEL);
51285148
if (!old_opts.s_qf_names[i]) {
51295149
for (j = 0; j < i; j++)
51305150
kfree(old_opts.s_qf_names[j]);
@@ -5353,9 +5373,12 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
53535373
#ifdef CONFIG_QUOTA
53545374
sbi->s_jquota_fmt = old_opts.s_jquota_fmt;
53555375
for (i = 0; i < EXT4_MAXQUOTAS; i++) {
5356-
kfree(sbi->s_qf_names[i]);
5357-
sbi->s_qf_names[i] = old_opts.s_qf_names[i];
5376+
to_free[i] = get_qf_name(sb, sbi, i);
5377+
rcu_assign_pointer(sbi->s_qf_names[i], old_opts.s_qf_names[i]);
53585378
}
5379+
synchronize_rcu();
5380+
for (i = 0; i < EXT4_MAXQUOTAS; i++)
5381+
kfree(to_free[i]);
53595382
#endif
53605383
kfree(orig_data);
53615384
return err;
@@ -5546,7 +5569,7 @@ static int ext4_write_info(struct super_block *sb, int type)
55465569
*/
55475570
static int ext4_quota_on_mount(struct super_block *sb, int type)
55485571
{
5549-
return dquot_quota_on_mount(sb, EXT4_SB(sb)->s_qf_names[type],
5572+
return dquot_quota_on_mount(sb, get_qf_name(sb, EXT4_SB(sb), type),
55505573
EXT4_SB(sb)->s_jquota_fmt, type);
55515574
}
55525575

0 commit comments

Comments
 (0)