From: Jan Kara <jack@suse.cz>
To: Chen Gang <gang.chen@asianux.com>
Cc: Theodore Ts'o <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
akpm@linux-foundation.org, linux-ext4@vger.kernel.org
Subject: Re: [PATCH] ext4: Fix memory leak when quota options are specified multiple times
Date: Mon, 21 Jan 2013 11:31:38 +0100 [thread overview]
Message-ID: <20130121103138.GB5588@quack.suse.cz> (raw)
In-Reply-To: <50FBC2D8.9070807@asianux.com>
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@suse.cz>
Honza
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> Cc: Jan Kara <jack@suse.cz>
> ---
> 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@suse.cz>
SUSE Labs, CR
next prev parent reply other threads:[~2013-01-21 10:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-20 10:11 [PATCH] ext4: Fix memory leak when quota options are specified multiple times Chen Gang
2013-01-21 10:31 ` Jan Kara [this message]
2013-01-21 10:59 ` Chen Gang
2013-01-21 11:05 ` Jan Kara
2013-01-21 11:07 ` Chen Gang
2013-01-25 3:49 ` Theodore Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130121103138.GB5588@quack.suse.cz \
--to=jack@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=gang.chen@asianux.com \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).