From: Brian Norris <computersforpeace@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
Frans Klaver <fransklaver@gmail.com>,
linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.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 00:32:09 +0000 [thread overview]
Message-ID: <20160716003209.GC76613@google.com> (raw)
In-Reply-To: <20160715110629.GB9258@mwanda>
+ 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.)
Regards,
Brian
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/mtd/maps/sa1100-flash.c b/drivers/mtd/maps/sa1100-flash.c
> index 142fc3d..784c6e1 100644
> --- a/drivers/mtd/maps/sa1100-flash.c
> +++ b/drivers/mtd/maps/sa1100-flash.c
> @@ -230,8 +230,10 @@ static struct sa_info *sa1100_setup_mtd(struct platform_device *pdev,
>
> info->mtd = mtd_concat_create(cdev, info->num_subdev,
> plat->name);
> - if (info->mtd = NULL)
> + if (info->mtd = NULL) {
> ret = -ENXIO;
> + goto err;
> + }
> }
> info->mtd->dev.parent = &pdev->dev;
>
[0] I haven't tried to prove that all patches with 'Fixes' tags go to
the -stable queue, but I know at least that this commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id;5394a3ccffbfa1d1d448d48742853a862822c4
ended up in v4.5.y here:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id€0a0b8a973b4262c92c228043cd17455cdf1a15
and IIRC, there are plenty more like that.
[1] Documentation/stable_kernel_rules.txt
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
Frans Klaver <fransklaver@gmail.com>,
linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.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 17:32:09 -0700 [thread overview]
Message-ID: <20160716003209.GC76613@google.com> (raw)
In-Reply-To: <20160715110629.GB9258@mwanda>
+ 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.)
Regards,
Brian
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/mtd/maps/sa1100-flash.c b/drivers/mtd/maps/sa1100-flash.c
> index 142fc3d..784c6e1 100644
> --- a/drivers/mtd/maps/sa1100-flash.c
> +++ b/drivers/mtd/maps/sa1100-flash.c
> @@ -230,8 +230,10 @@ static struct sa_info *sa1100_setup_mtd(struct platform_device *pdev,
>
> info->mtd = mtd_concat_create(cdev, info->num_subdev,
> plat->name);
> - if (info->mtd == NULL)
> + if (info->mtd == NULL) {
> ret = -ENXIO;
> + goto err;
> + }
> }
> info->mtd->dev.parent = &pdev->dev;
>
[0] I haven't tried to prove that all patches with 'Fixes' tags go to
the -stable queue, but I know at least that this commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3b5394a3ccffbfa1d1d448d48742853a862822c4
ended up in v4.5.y here:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=800a0b8a973b4262c92c228043cd17455cdf1a15
and IIRC, there are plenty more like that.
[1] Documentation/stable_kernel_rules.txt
next prev parent reply other threads:[~2016-07-16 0:32 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 [this message]
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
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=20160716003209.GC76613@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.