linux-kernel - Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1440021013.2737.14.camel@kernel.crashing.org>
Date:	Thu, 20 Aug 2015 07:50:13 +1000
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Laurent Dufour <ldufour@...ux.vnet.ibm.com>,
	David Laight <David.Laight@...LAB.COM>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver

On Wed, 2015-08-19 at 23:37 +0200, Laurent Dufour wrote:
> On 03/08/2015 13:00, David Laight wrote:
From: Laurent Dufour
> > > Sent: 31 July 2015 10:30
> > > This patch fixes several endianness issues detected when running
> > > the HVSI
> > > driver in little endian mode.
> > > 
> > > These issues are raised in little endian mode because the data
> > > exchanged in
> > > memory between the kernel and the hypervisor has to be in big
> > > endian
> > > format.
> > ...
> > > diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
> > > index 41901997c0d6..a75146f600cb 100644
> > > --- a/drivers/tty/hvc/hvsi.c
> > > +++ b/drivers/tty/hvc/hvsi.c
> > > @@ -240,9 +240,9 @@ static void hvsi_recv_control(struct
> > > hvsi_struct *hp, uint8_t *packet,
> > >  {
> > >  	struct hvsi_control *header = (struct hvsi_control
> > > *)packet;
> > > 
> > > -	switch (header->verb) {
> > > +	switch (be16_to_cpu(header->verb)) {
> > >  		case VSV_MODEM_CTL_UPDATE:
> > > -			if ((header->word & HVSI_TSCD) == 0) {
> > > +			if ((be32_to_cpu(header->word) &
> > > HVSI_TSCD) == 0) {
> > 
> > It is generally best to byteswap constants.
> > 
> > 	David
> 
> Thanks David for your review.
> Regarding the byte swapping of the constants, I'm wondering if this 
>  the
> best way here.
> For instance, Benjamin wrote a similar patch to fix another
> endianness
> issue (99fc1d91b8fc) and he doesn't convert the constant neither.
> It think that byte swapping the constant value will impact more code,
> and may not ease code reading.

Right, I disagree with byteswapping the constants at their definition
point, however maybe he meant using cpu_to_be16(CONSTANT) ?

In any case, it's pretty moot as we have the lhbrx instruction which
will do the load and byteswap and for HVSI, even if it was a tad slower
than a normal load, it would not make a noticeable difference.

Ben.

--
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/