All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Frans Klaver <fransklaver@gmail.com>,
	linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mtd: maps: sa1100-flash: potential NULL dereference
Date: Sat, 16 Jul 2016 01:46:37 +0000	[thread overview]
Message-ID: <20160716014637.GE76613@google.com> (raw)
In-Reply-To: <20160716004825.GA10855@kroah.com>

Hi,

On Sat, Jul 16, 2016 at 09:48:25AM +0900, Greg Kroah-Hartman wrote:
> On Fri, Jul 15, 2016 at 05:32:09PM -0700, Brian Norris wrote:
> > + stable
> > 
> > Hi Dan,
> > 
> > Patch looks good, but one question.
> > 
> > On Fri, Jul 15, 2016 at 02:06:30PM +0300, Dan Carpenter wrote:
> > > We check for NULL but then dereference "info->mtd" on the next line.
> > > 
> > > Fixes: 72169755cf36 ('mtd: maps: sa1100-flash: show parent device in sysfs')
> > 
> > What am I supposed to do about tags like this? It appears that the
> > -stable folks have started taking patches with a 'Fixes' tag alone [0],
> > even though that's not mentioned in [1]. I ask because I strongly
> > suspect this patch doesn't fit the rules in [1] -- it quite likely has
> > only been compile tested; and it qualifies quite well as violating
> > bullet 4:
> > 
> > """
> >  - It must fix a real bug that bothers people (not a, "This could be a
> >    problem..." type thing).
> > """
> > 
> > So, I'd like to keep the tag, but I'd like to avoid having to NAK it in
> > the stable review process. (And really, I often don't care enough to
> > even do that. I believe there's a very low chance that something like
> > this would cause additional problems more than the original bug.)
> 
> Only sometimes will I pick up something that only has a fixes: tag in
> it, not all the time, I try to review the patch to see if it does match
> the rules or not.

OK, good to know. I've seen other -stable maintainers do similarly, but
I don't know what their process is.

> But, fixing an oops is a good thing, I'm sure you can figure out how to
> trigger it otherwise you would not be taking such a patch as it would be
> not be needed :)

Of course. But it's still not always clear whether such fixes will
trigger other errors in poorly-tested error paths. Is (for instance) an
oops that we know about better than a use-after-free that we don't know
about?

Anyway, applied to l2-mtd.git.

Regards,
Brian

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Frans Klaver <fransklaver@gmail.com>,
	linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mtd: maps: sa1100-flash: potential NULL dereference
Date: Fri, 15 Jul 2016 18:46:37 -0700	[thread overview]
Message-ID: <20160716014637.GE76613@google.com> (raw)
In-Reply-To: <20160716004825.GA10855@kroah.com>

Hi,

On Sat, Jul 16, 2016 at 09:48:25AM +0900, Greg Kroah-Hartman wrote:
> On Fri, Jul 15, 2016 at 05:32:09PM -0700, Brian Norris wrote:
> > + stable
> > 
> > Hi Dan,
> > 
> > Patch looks good, but one question.
> > 
> > On Fri, Jul 15, 2016 at 02:06:30PM +0300, Dan Carpenter wrote:
> > > We check for NULL but then dereference "info->mtd" on the next line.
> > > 
> > > Fixes: 72169755cf36 ('mtd: maps: sa1100-flash: show parent device in sysfs')
> > 
> > What am I supposed to do about tags like this? It appears that the
> > -stable folks have started taking patches with a 'Fixes' tag alone [0],
> > even though that's not mentioned in [1]. I ask because I strongly
> > suspect this patch doesn't fit the rules in [1] -- it quite likely has
> > only been compile tested; and it qualifies quite well as violating
> > bullet 4:
> > 
> > """
> >  - It must fix a real bug that bothers people (not a, "This could be a
> >    problem..." type thing).
> > """
> > 
> > So, I'd like to keep the tag, but I'd like to avoid having to NAK it in
> > the stable review process. (And really, I often don't care enough to
> > even do that. I believe there's a very low chance that something like
> > this would cause additional problems more than the original bug.)
> 
> Only sometimes will I pick up something that only has a fixes: tag in
> it, not all the time, I try to review the patch to see if it does match
> the rules or not.

OK, good to know. I've seen other -stable maintainers do similarly, but
I don't know what their process is.

> But, fixing an oops is a good thing, I'm sure you can figure out how to
> trigger it otherwise you would not be taking such a patch as it would be
> not be needed :)

Of course. But it's still not always clear whether such fixes will
trigger other errors in poorly-tested error paths. Is (for instance) an
oops that we know about better than a use-after-free that we don't know
about?

Anyway, applied to l2-mtd.git.

Regards,
Brian

  reply	other threads:[~2016-07-16  1:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-15 11:06 [patch] mtd: maps: sa1100-flash: potential NULL dereference Dan Carpenter
2016-07-15 11:06 ` Dan Carpenter
2016-07-16  0:32 ` Brian Norris
2016-07-16  0:32   ` Brian Norris
2016-07-16  0:48   ` Greg Kroah-Hartman
2016-07-16  0:48     ` Greg Kroah-Hartman
2016-07-16  1:46     ` Brian Norris [this message]
2016-07-16  1:46       ` Brian Norris
2016-07-16  9:00   ` Dan Carpenter
2016-07-16  9:00     ` Dan Carpenter
2016-07-17  3:54     ` Brian Norris
2016-07-17  3:54       ` Brian Norris

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=20160716014637.GE76613@google.com \
    --to=computersforpeace@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=dwmw2@infradead.org \
    --cc=fransklaver@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=stable@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.