linux-kernel - Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 13 Sep 2016 17:24:23 -0500
From:   Scott Wood <oss@...error.net>
To:     "Y.B. Lu" <yangbo.lu@....com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        Arnd Bergmann <arnd@...db.de>
Cc:     "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Russell King <linux@....linux.org.uk>,
        Jochen Friedrich <jochen@...am.de>,
        Joerg Roedel <joro@...tes.org>,
        Claudiu Manoil <claudiu.manoil@...escale.com>,
        Bhupesh Sharma <bhupesh.sharma@...escale.com>,
        Qiang Zhao <qiang.zhao@....com>,
        Kumar Gala <galak@...eaurora.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Leo Li <leoyang.li@....com>, "X.B. Xie" <xiaobo.xie@....com>
Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms

On Tue, 2016-09-13 at 07:23 +0000, Y.B. Lu wrote:
> > 


> > 
> > -----Original Message-----
> > From: linux-mmc-owner@...r.kernel.org [mailto:linux-mmc-
> > owner@...r.kernel.org] On Behalf Of Scott Wood
> > Sent: Tuesday, September 13, 2016 7:25 AM
> > To: Y.B. Lu; linux-mmc@...r.kernel.org; ulf.hansson@...aro.org; Arnd
> > Bergmann
> > Cc: linuxppc-dev@...ts.ozlabs.org; devicetree@...r.kernel.org; linux-arm-
> > kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; linux-
> > clk@...r.kernel.org; linux-i2c@...r.kernel.org; iommu@...ts.linux-
> > foundation.org; netdev@...r.kernel.org; Mark Rutland; Rob Herring;
> > Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> > Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie
> > Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
> > 
> > BTW, aren't ls2080a and ls2085a the same die?  And is there no non-E
> > version of LS2080A/LS2040A?
> [Lu Yangbo-B47093] I checked all the svr values in chip errata doc "Revision
> level to part marking cross-reference" table.
> I found ls2080a and ls2085a were in two separate doc. And I didn’t find non-
> E version of LS2080A/LS2040A in chip errata doc.
> Do you know is there any other doc we can confirm this?

No.  Traditionally we've always had E and non-E versions of each chip, but I
have no knowledge of whether that has changed (I do note that the way that E-
status is indicated in SVR has changed).

But please label LS2080A and LS2085A as the same die (or provide strong
evidence that they are not).

> 
> > 
> > 
> > > 
> > > > > 
> > > > > +	do {
> > > > > +		if (!matches->soc_id)
> > > > > +			return NULL;
> > > > > +		if (glob_match(svr_match, matches->soc_id))
> > > > > +			break;
> > > > > +	} while (matches++);
> > > > Are you expecting "matches++" to ever evaluate as false?
> > > [Lu Yangbo-B47093] Yes, this is used to match the soc we use in
> > > qoriq_soc array until getting true.
> > > We need to get the name and die information defined in array.
> > I'm not asking whether the glob_match will ever return true.  I'm saying
> > that "matches++" will never become NULL.
> [Lu Yangbo-B47093] The matches++ will never become NULL while it will return
> NULL after matching for all the members in array.

"matches++" will never "return NULL".  It's just an incrementing address.  It
won't be null until you wrap around the address space, and even if the other
loop terminators never kicked in you'd crash long before that happens.

Please rewrite the loop as something like:

	while (matches->soc_id) {
		if (glob_match(...))
			return matches;

		matches++;
	}

	return NULL;


> > > > > +	/* Register soc device */
> > > > > +	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > > > > +	if (!soc_dev_attr) {
> > > > > +		ret = -ENOMEM;
> > > > > +		goto out_unmap;
> > > > > +	}
> > > > Couldn't this be statically allocated?
> > > [Lu Yangbo-B47093] Do you mean we define this struct statically ?
> > > 
> > > static struct soc_device_attribute soc_dev_attr;
> > Yes.
> > 
> [Lu Yangbo-B47093] It's ok to define it statically. Is there any need to do
> that?

It's simpler.

-Scott