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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 19 Sep 2014 13:46:04 +0800
From:	Dong Aisheng <b29396@...escale.com>
To:	Xiubo Li-B47053 <Li.Xiubo@...escale.com>
CC:	Dong Aisheng-B29396 <Aisheng.Dong@...escale.com>,
	Pankaj Dubey <pankaj.dubey@...sung.com>,
	"arnd@...db.de" <arnd@...db.de>,
	"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>,
	"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

On Fri, Sep 19, 2014 at 01:20:18PM +0800, Xiubo Li-B47053 wrote:
> [...]
> > >    create child: /dcsr@...00000/dcsr-atbrepl@...000
> > >    create child: /dcsr@...00000/dcsr-tsgen-ctrl@...000
> > >    create child: /dcsr@...00000/dcsr-tsgen-read@...000
> > >    create child: /regulators/regulator@0
> > >    ...
> > >
> > > As default the Linux will create all the platform device for each DT node,
> > which
> > > Can be found from "drivers/of/platform.c".
> > >
> > > So we can get the pdev node using the specified DT node, and feel safe to
> > use
> > > it as Pankaj's patch does.
> > >
> > 
> > I mean before the devices are populated from device tree.
> > For example, we usually call of_platform_populate in .init_machine.
> > Before it, we may not be able to get it's device, isn't it?
> > 
> 
> Yes, right.
> 
> For this case, we'd better create the pdev or dev manually for the first time
> We use it, right ?

Yes, that's my understanding.

Regards
Dong Aisheng

> 
> Thanks,
> 
> BRs
> Xiubo
> 
> 
> > Regards
> > Dong Aisheng
> > 
> > > And also we must make sure that the 'syscon' DT nodes has the compatible
> > prop.
> > >
> > > Thanks,
> > >
> > > BRs
> > > Xiubo
> > >
> > >
> > > > Regards
> > > > Dong Aisheng
> > > >
> > > > > ----
> > > > > static struct syscon *of_syscon_register(struct device_node *np)
> > > > >  {
> > > > > +	struct platform_device *pdev;
> > > > >  	struct syscon *syscon;
> > > > >  	struct regmap *regmap;
> > > > >  	void __iomem *base;
> > > > > @@ -142,7 +144,11 @@ static struct syscon *of_syscon_register(struct
> > > > > device_node *np)
> > > > >  	if (!base)
> > > > >  		return ERR_PTR(-ENOMEM);
> > > > >
> > > > > -	regmap = regmap_init_mmio(NULL, base, &syscon_regmap_config);
> > > > > +	pdev = of_find_device_by_node(np);
> > > > > +	if (!(&pdev->dev))
> > > > > +		return ERR_PTR(-ENODEV);
> > > > > +
> > > > > +	regmap = regmap_init_mmio(&pdev->dev, base,
> > &syscon_regmap_config);
> > > > >  	if (IS_ERR(regmap)) {
> > > > >  		pr_err("regmap init failed\n");
> > > > >  		return ERR_CAST(regmap);
> > > > > -------
> > > > >
> > > > > I have tested this in linux-next and it works well. In this way there
> > won't
> > > > > be any issues of
> > > > > dereferencing NULL pointer in regmap.c and at the same time, if DT has
> > > > > {big,little}-endian
> > > > > optional property in syscon device node, it will be taken care.
> > > > >
> > > > > So I would wait for Arnd's opinion about above mentioned changes and
> > then
> > > > > post a new
> > > > > change after addressing Arnd's minor comment along with this fix in next
> > > > > revision.
> > > > >
> > > > >
> > > > > Thanks,
> > > > > Pankaj Dubey
> > > > > > Maybe we could consider create device structure for each syscon
> > compatible
> > > > > device in
> > > > > > syscon driver in of_syscon_register in first time which seems to be
> > > > > reasonable.
> > > > > >
> > > > > > Regards
> > > > > > Dong Aisheng
> > > > > >
> > > > > > > --------------------------------------------
> > > > > > > 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