| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c93278a6da2b40649089426026a857f7@BY2PR0301MB0613.namprd03.prod.outlook.com>
Date: Thu, 18 Sep 2014 06:33:24 +0000
From: "Li.Xiubo@...escale.com" <Li.Xiubo@...escale.com>
To: Pankaj Dubey <pankaj.dubey@...sung.com>,
"Aisheng.Dong@...escale.com" <Aisheng.Dong@...escale.com>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kgene.kim@...sung.com" <kgene.kim@...sung.com>,
"linux@....linux.org.uk" <linux@....linux.org.uk>,
"arnd@...db.de" <arnd@...db.de>,
"naushad@...sung.com" <naushad@...sung.com>,
"tomasz.figa@...il.com" <tomasz.figa@...il.com>,
"joshi@...sung.com" <joshi@...sung.com>,
"thomas.ab@...sung.com" <thomas.ab@...sung.com>,
"vikas.sajjan@...sung.com" <vikas.sajjan@...sung.com>,
"chow.kim@...sung.com" <chow.kim@...sung.com>,
"lee.jones@...aro.org" <lee.jones@...aro.org>,
"'Boris BREZILLON'" <boris.brezillon@...e-electrons.com>,
'Geert Uytterhoeven' <geert+renesas@...der.be>,
'Stephen Warren' <swarren@...dia.com>
Subject: RE: [PATCH v3] mfd: syscon: Decouple syscon interface from platform
devices
Hi Pankaj,
One more question:
For example:
regmap_read()
----> _regmap_read()
----> 2112 #ifdef LOG_DEVICE
----> 2113 if (strcmp(dev_name(map->dev), LOG_DEVICE) == 0)
----> 2114 dev_info(map->dev, "%x => %x\n", reg, *val);
----> 2115 #endif
In dev_name(map->dev) will also encounter the same crash core trace like in
regmap_get_val_endian(dev, ...).
Could there be another method, such as using 'dummy dev' instead of 'NULL dev' ?
Just one suggestion.
BRs
Xiubo
> -----Original Message-----
> From: Pankaj Dubey [mailto:pankaj.dubey@...sung.com]
> Sent: Thursday, September 18, 2014 2:03 PM
> To: Dong Aisheng-B29396
> Cc: linux-arm-kernel@...ts.infradead.org; linux-samsung-soc@...r.kernel.org;
> linux-kernel@...r.kernel.org; kgene.kim@...sung.com; linux@....linux.org.uk;
> arnd@...db.de; naushad@...sung.com; tomasz.figa@...il.com; joshi@...sung.com;
> thomas.ab@...sung.com; vikas.sajjan@...sung.com; chow.kim@...sung.com;
> lee.jones@...aro.org; 'Boris BREZILLON'; Xiubo Li-B47053; 'Geert Uytterhoeven';
> 'Stephen Warren'
> Subject: RE: [PATCH v3] mfd: syscon: Decouple syscon interface from platform
> devices
>
> Hi,
>
> Adding CC to Xiubo Li, Geert Uytterhoeven and Stephen Warren.
>
> On Thursday, September 18, 2014, Dong Aisheng wrote,
> > On Wed, Sep 17, 2014 at 04:50:50PM +0530, Pankaj Dubey wrote:
> > > Hi,
> > >
> > > On Wednesday, September 17, 2014, Dong Aisheng Wrote,
> > > > >
> > > > > + regmap = regmap_init_mmio(NULL, base,
> &syscon_regmap_config);
> > > >
> > > > Does a NULL device pointer work?
> > >
> > > Yes, it is safe, at least we are able to test on Exynos based SoC.
> > > I have tested it with kgene/for-next kernel on Exynos3250.
> > > Also it has been tested on Exynos5250 based Snow board with 3.17-rc5
> > > based kernel by Vivek Gautam.
> > >
> > > Patch V2 also has been tested by "Borris Brezillon" on AT91 platform.
> > >
> > >
> >
> > The kernel i tested was next-20140915 of linux-next.
> >
> > please see regmap_get_val_endian called in regmap_init function.
> > static enum regmap_endian regmap_get_val_endian(struct device *dev,
> > const struct regmap_bus *bus,
> > const struct regmap_config
> *config) {
> > struct device_node *np = dev->of_node;
> > enum regmap_endian endian;
> > ...
> > }
> > It will crash at the first line of dev->of_node if dev is NULL.
> >
> > Can you check if you're using the same code as mine?
>
> No, it's not same.
> My bad that I was not using linux-next for testing this patch.
> We tested on kgene/for-next where these changes still have not come.
> Just now I checked linux-next and found that it will crash at first line of
> "regmap_get_val_endian" as there is no check for NULL on dev.
>
> I checked git history of regmap.c file and found recently this file has been
> modified
> for adding DT endianness binding support. Following are set of patches gone
> for this
>
> cf673fb regmap: Split regmap_get_endian() in two functions
> 5844a8b regmap: Fix handling of volatile registers for format_write() chips
> 45e1a27 regmap: of_regmap_get_endian() cleanup
> ba1b53f regmap: Fix DT endianess parsing logic
> d647c19 regmap: add DT endianness binding support.
>
> I think there should have been a check for NULL on "dev" in
> "regmap_get_val_endian", so that if dev pointer exist then only it makes
> sense to get
> endianness property from DT.
>
> I will suggest following fix in regmap.c for this. With following fix I
> tested it and it works well
> on linux-next also. So if you can confirm following fix is working for you
> then I can post this
> patch.
>
> --------------------------------------------
> Subject: [PATCH] regmap: fix NULL pointer dereference in
> regmap_get_val_endian
>
> Recent commits for getting reg endianess causing NULL pointer
> dereference if dev is passed NULL in regmap_init_mmio. This patch
> fixes this issue, and allows to parse reg endianess only if dev
> and dev->of_node exist.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@...sung.com>
> ---
> drivers/base/regmap/regmap.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index f2281af..455a877 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -477,7 +477,7 @@ static enum regmap_endian regmap_get_val_endian(struct
> device *dev,
> const struct regmap_bus *bus,
> const struct regmap_config *config)
> {
> - struct device_node *np = dev->of_node;
> + struct device_node *np;
> enum regmap_endian endian;
>
> /* Retrieve the endianness specification from the regmap config */
> @@ -487,15 +487,20 @@ static enum regmap_endian regmap_get_val_endian(struct
> device *dev,
> if (endian != REGMAP_ENDIAN_DEFAULT)
> return endian;
>
> - /* Parse the device's DT node for an endianness specification */
> - if (of_property_read_bool(np, "big-endian"))
> - endian = REGMAP_ENDIAN_BIG;
> - else if (of_property_read_bool(np, "little-endian"))
> - endian = REGMAP_ENDIAN_LITTLE;
> + /* If the dev and dev->of_node exist try to get endianness from DT
> */
> + if (dev && dev->of_node) {
> + np = dev->of_node;
>
> - /* If the endianness was specified in DT, use that */
> - if (endian != REGMAP_ENDIAN_DEFAULT)
> - return endian;
> + /* Parse the device's DT node for an endianness
> specification */
> + if (of_property_read_bool(np, "big-endian"))
> + endian = REGMAP_ENDIAN_BIG;
> + else if (of_property_read_bool(np, "little-endian"))
> + endian = REGMAP_ENDIAN_LITTLE;
> +
> + /* If the endianness was specified in DT, use that */
> + if (endian != REGMAP_ENDIAN_DEFAULT)
> + return endian;
> + }
>
> /* Retrieve the endianness specification from the bus config */
> if (bus && bus->val_format_endian_default)
> --
>
> Thanks,
> Pankaj Dubey
>
> > Regards
> > Dong Aisheng
> >
> > >
> > > Thanks,
> > > Pankaj Dubey
> > >
> > > > Regards
> > > > Dong Aisheng
> > > >
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > linux-arm-kernel mailing list
> > > > > linux-arm-kernel@...ts.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists