linux-kernel - Re: [PATCH] usb: dwc2: fix regression on big-endian PowerPC/ARM systems

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2809110.sWekaCNVxS@wuerfel>
Date:	Thu, 12 May 2016 13:36:40 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Felipe Balbi <felipe.balbi@...ux.intel.com>
Cc:	Christian Lamparter <chunkeey@...glemail.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linux-mips@...ux-mips.org, johnyoun@...opsys.com,
	gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, a.seppala@...il.com,
	linuxppc-dev@...ts.ozlabs.org, Felipe Balbi <balbi@...com>,
	Douglas Anderson <dianders@...omium.org>,
	Gregory Herrero <gregory.herrero@...el.com>,
	Mian Yousaf Kaukab <yousaf.kaukab@...el.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH] usb: dwc2: fix regression on big-endian PowerPC/ARM systems

On Thursday 12 May 2016 14:25:49 Felipe Balbi wrote:
> >  {
> >       u32 value = __raw_readl(addr);
> >  
> > -     /* In order to preserve endianness __raw_* operation is used. Therefore
> > -      * a barrier is needed to ensure IO access is not re-ordered across
> > +     /* in order to preserve endianness __raw_* operation is used. therefore
> > +      * a barrier is needed to ensure io access is not re-ordered across
> >        * reads or writes
> >        */
> >       mb();
> > @@ -81,15 +93,32 @@ static inline void dwc2_writel(u32 value, void __iomem *addr)
> >       __raw_writel(value, addr);
> >  
> >       /*
> > -      * In order to preserve endianness __raw_* operation is used. Therefore
> > -      * a barrier is needed to ensure IO access is not re-ordered across
> > +      * in order to preserve endianness __raw_* operation is used. therefore
> > +      * a barrier is needed to ensure io access is not re-ordered across
> >        * reads or writes
> >        */
> >       mb();
> > -#ifdef DWC2_LOG_WRITES
> > -     pr_info("INFO:: wrote %08x to %p\n", value, addr);
> > +#ifdef dwc2_log_writes
> > +     pr_info("info:: wrote %08x to %p\n", value, addr);
> >  #endif
> >  }
> > +#else

Oops, the accidental lowercase conversion is still in here, I'll fix it
up once we agree on the approach.

> I still think this is something that should be handled at MIPS side, no ?

As I explained, there isn't really anything we can do in MIPS code
because of the way they have to handle PCI.

> How many more drivers will we have to 'fix' like this ?

Endianess problems will keep coming up, and we have hundreds or thousands
of drivers that are written with a particular design in mind that could
be wrong as soon as someone chooses to build an SoC that does things
differently. Once that happens, we'll fix them.

Also, Christian has already posted a better version of the patch
that fixes this driver in an architecture independent way, but we still
need a workaround for the stable backports.

	Arnd