linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: device-mapper development <dm-devel@redhat.com>,
	linux-kernel@vger.kernel.org, Neil Brown <neilb@suse.com>,
	linux-raid@vger.kernel.org,
	Sudip Mukherjee <sudipm.mukherjee@gmail.com>,
	Alasdair Kergon <agk@redhat.com>
Subject: Re: dm: ioctl: prevent double freeing
Date: Tue, 15 Sep 2015 14:13:33 -0400	[thread overview]
Message-ID: <20150915181333.GB18972@redhat.com> (raw)
In-Reply-To: <20150915161855.GE10399@birch.djwong.org>

On Tue, Sep 15 2015 at 12:18pm -0400,
Darrick J. Wong <darrick.wong@oracle.com> wrote:

> On Fri, Sep 04, 2015 at 04:52:39PM +0530, Sudip Mukherjee wrote:
> > DM_PARAMS_KMALLOC and DM_PARAMS_VMALLOC should never be set together in
> > param_flags. We are setting these flags while allocating so we know that
> > there is almost no chance of having these two set together but still we
> > can have some additional safety.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> >  drivers/md/dm-ioctl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> > index 80a4395..aaad74e 100644
> > --- a/drivers/md/dm-ioctl.c
> > +++ b/drivers/md/dm-ioctl.c
> > @@ -1680,7 +1680,7 @@ static void free_params(struct dm_ioctl *param, size_t param_size, int param_fla
> >  
> >  	if (param_flags & DM_PARAMS_KMALLOC)
> >  		kfree(param);
> > -	if (param_flags & DM_PARAMS_VMALLOC)
> > +	else if (param_flags & DM_PARAMS_VMALLOC)
> 
> Wouldn't you also want a WARN_ON to complain about both flags being set?

No, this is getting rediculous now.  Please stop the insanity.

DM_PARAMS_KMALLOC and DM_PARAMS_VMALLOC are mutually exclussive --
always will be.  Sprinkling defensive code around isn't high on my list
of changes I'm interested in.

At some point I'll stage the original change from Sudip for 4.4 but I'm
re-writing the subject and header to not be so alarmist (this is more to
do with eliminating a conditional than anything else).  There is _zero_
potential for a double-free with the current code.

Mike

      reply	other threads:[~2015-09-15 18:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-04 11:22 [PATCH] dm: ioctl: prevent double freeing Sudip Mukherjee
2015-09-15  7:53 ` Sudip Mukherjee
2015-09-15 16:18 ` [dm-devel] " Darrick J. Wong
2015-09-15 18:13   ` Mike Snitzer [this message]

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=20150915181333.GB18972@redhat.com \
    --to=snitzer@redhat.com \
    --cc=agk@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.com \
    --cc=sudipm.mukherjee@gmail.com \
    /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).