ext4: fix memory leak when quota options are specified multiple times
authorChen Gang <gang.chen@asianux.com>
Fri, 25 Jan 2013 04:24:58 +0000 (23:24 -0500)
committerTheodore Ts'o <tytso@mit.edu>
Fri, 25 Jan 2013 04:24:58 +0000 (23:24 -0500)
When usrjquota or grpjquota mount options are specified several times,
we leak memory storing the names. Free the memory correctly.

Signed-off-by: Chen Gang <gang.chen@asianux.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Reviewed-by: Jan Kara <jack@suse.cz>
fs/ext4/super.c

index 3ac306064b285c88707ef97dfe3faebc62d95be6..3b04d15f303d69ba24a9e81bac5837fca155b788 100644 (file)
@@ -1337,6 +1337,7 @@ static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
 {
        struct ext4_sb_info *sbi = EXT4_SB(sb);
        char *qname;
+       int ret = -1;
 
        if (sb_any_quota_loaded(sb) &&
                !sbi->s_qf_names[qtype]) {
@@ -1351,23 +1352,26 @@ static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
                        "Not enough memory for storing quotafile name");
                return -1;
        }
-       if (sbi->s_qf_names[qtype] &&
-               strcmp(sbi->s_qf_names[qtype], qname)) {
-               ext4_msg(sb, KERN_ERR,
-                       "%s quota file already specified", QTYPE2NAME(qtype));
-               kfree(qname);
-               return -1;
+       if (sbi->s_qf_names[qtype]) {
+               if (strcmp(sbi->s_qf_names[qtype], qname) == 0)
+                       ret = 1;
+               else
+                       ext4_msg(sb, KERN_ERR,
+                                "%s quota file already specified",
+                                QTYPE2NAME(qtype));
+               goto errout;
        }
-       sbi->s_qf_names[qtype] = qname;
-       if (strchr(sbi->s_qf_names[qtype], '/')) {
+       if (strchr(qname, '/')) {
                ext4_msg(sb, KERN_ERR,
                        "quotafile must be on filesystem root");
-               kfree(sbi->s_qf_names[qtype]);
-               sbi->s_qf_names[qtype] = NULL;
-               return -1;
+               goto errout;
        }
+       sbi->s_qf_names[qtype] = qname;
        set_opt(sb, QUOTA);
        return 1;
+errout:
+       kfree(qname);
+       return ret;
 }
 
 static int clear_qf_name(struct super_block *sb, int qtype)
@@ -1381,10 +1385,7 @@ static int clear_qf_name(struct super_block *sb, int qtype)
                        " when quota turned on");
                return -1;
        }
-       /*
-        * The space will be released later when all options are confirmed
-        * to be correct
-        */
+       kfree(sbi->s_qf_names[qtype]);
        sbi->s_qf_names[qtype] = NULL;
        return 1;
 }
@@ -4593,7 +4594,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
        unsigned int journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
        int err = 0;
 #ifdef CONFIG_QUOTA
-       int i;
+       int i, j;
 #endif
        char *orig_data = kstrdup(data, GFP_KERNEL);
 
@@ -4609,7 +4610,16 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 #ifdef CONFIG_QUOTA
        old_opts.s_jquota_fmt = sbi->s_jquota_fmt;
        for (i = 0; i < MAXQUOTAS; i++)
-               old_opts.s_qf_names[i] = sbi->s_qf_names[i];
+               if (sbi->s_qf_names[i]) {
+                       old_opts.s_qf_names[i] = kstrdup(sbi->s_qf_names[i],
+                                                        GFP_KERNEL);
+                       if (!old_opts.s_qf_names[i]) {
+                               for (j = 0; j < i; j++)
+                                       kfree(old_opts.s_qf_names[j]);
+                               return -ENOMEM;
+                       }
+               } else
+                       old_opts.s_qf_names[i] = NULL;
 #endif
        if (sbi->s_journal && sbi->s_journal->j_task->io_context)
                journal_ioprio = sbi->s_journal->j_task->io_context->ioprio;
@@ -4742,9 +4752,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 #ifdef CONFIG_QUOTA
        /* Release old quota file names */
        for (i = 0; i < MAXQUOTAS; i++)
-               if (old_opts.s_qf_names[i] &&
-                   old_opts.s_qf_names[i] != sbi->s_qf_names[i])
-                       kfree(old_opts.s_qf_names[i]);
+               kfree(old_opts.s_qf_names[i]);
        if (enable_quota) {
                if (sb_any_quota_suspended(sb))
                        dquot_resume(sb, -1);
@@ -4773,9 +4781,7 @@ restore_opts:
 #ifdef CONFIG_QUOTA
        sbi->s_jquota_fmt = old_opts.s_jquota_fmt;
        for (i = 0; i < MAXQUOTAS; i++) {
-               if (sbi->s_qf_names[i] &&
-                   old_opts.s_qf_names[i] != sbi->s_qf_names[i])
-                       kfree(sbi->s_qf_names[i]);
+               kfree(sbi->s_qf_names[i]);
                sbi->s_qf_names[i] = old_opts.s_qf_names[i];
        }
 #endif