Re: [PATCH] mtdinfo: don't open NULL pointer when getting region_info with `-a'

Brian Foster
2011-08-05T07:30:51Z

On Thursday 04 August 2011 19:47:31 Brian Norris wrote:
> This "fixes" a regression found in:
> commit 266061ebd5d72391f0a0e831b018e8fc7fea68a1
> mtdinfo: add regioninfo/eraseblock map display
>
> On certain flash (NOR flash that have eraseblock region info),
> `mtdinfo -a' tries to open the MTD node file, for use with the ioctl
> MEMGETREGIONINFO; however, we didn't supply a device node path to
> `mtdinfo -a', so it's using NULL, resulting in errors like:
>
> mtdinfo: error!: couldn't open MTD dev: (null)
> error 14 (Bad address)
>
> For now, we can just skip dumping region_info with the `-a' flag. If
> we find a better way to do this (e.g., export via sysfs, find device
> nodes via automatic routines, etc.), then we can kill the workaround
> and this FIXME should be removed.
>
> The regression was first reported at:
>
> http://lists.infradead.org/pipermail/linux-mtd/2011-July/037232.html
>
> Reported-by: Brian Foster <brian.foster@maxim-ic.com>
> CC: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> ubi-utils/mtdinfo.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ubi-utils/mtdinfo.c b/ubi-utils/mtdinfo.c
> index e72d69e..f2fcd38 100644
> --- a/ubi-utils/mtdinfo.c
> +++ b/ubi-utils/mtdinfo.c
> @@ -239,8 +239,14 @@ static void print_region_info(const struct mtd_dev_info *mtd)
> region_info_t reginfo;
> int r, fd;
>
> - /* If we don't have any region info, just return */
> - if (!args.map && mtd->region_cnt == 0)
> + /*
> + * If we don't have any region info, just return
> + *
> + * FIXME: We can't get region_info (via ioctl) without having the MTD
> + * node path. This is a problem for `mtdinfo -a', for example,
> + * since it doesn't provide any filepath information.
> + */
> + if (!args.node || !args.map && mtd->region_cnt == 0)

I suggest adding some round brackets:

if (!args.node || (!args.map && mtd->region_cnt == 0))

Less confusing. Otherwise, looks Ok
to me (not actually built or tested).
cheers!
-blf-

> return;
>
> /* First open the device so we can query it */
> --
> 1.7.0.4

--
Brian FOSTER
Principal MTS, Software
Maxim Integrated Products (Microcontroller BU), formerly Innova Card
Web : http://www.maxim-ic.com/