On Sun 20-01-13 18:11:36, Chen Gang wrote:
>
> When usrjquota or grpjquota mount options are specified several times,
> we leak memory storing the names. Free the memory correctly.
The port looks good. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
>
> Signed-off-by: Chen Gang <gang.chen@xxxxxxxxxxx>
> Cc: Jan Kara <jack@xxxxxxx>
> ---
> fs/ext4/super.c | 48 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c014edd..a4a8ecc 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1351,21 +1351,25 @@ 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));
> + if (sbi->s_qf_names[qtype]) {
> + int ret = 1;
> +
> + if (strcmp(sbi->s_qf_names[qtype], qname)) {
> + ext4_msg(sb, KERN_ERR,
> + "%s quota file already specified",
> + QTYPE2NAME(qtype));
> + ret = -1;
> + }
> kfree(qname);
> - return -1;
> + return ret;
> }
> - 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;
> + kfree(qname);
> return -1;
> }
> + sbi->s_qf_names[qtype] = qname;
> set_opt(sb, QUOTA);
> return 1;
> }
> @@ -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;
> }
> @@ -4605,7 +4606,18 @@ 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]) {
> + int j;
> +
> + 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;
> @@ -4738,9 +4750,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);
> @@ -4769,9 +4779,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
> --
> 1.7.10.4
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
