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>] [day] [month] [year] [list]
Date: Wed, 10 Jan 2024 10:25:30 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Ariel Silver <arielsilver77@...il.com>
Cc: forest@...ttletooquiet.net, gregkh@...uxfoundation.org,
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging: vt6655: Fix sparse warning. Restricted cast.

Hm...  This stuff is more broken than I realized when I first looked at
it.  And the truth is that I don't know the hardware so I can't say
exactly what the fix is.  Probably best to just leave it alone for now...

On Wed, Jan 10, 2024 at 08:25:33AM +0200, Ariel Silver wrote:
> >> Hello Dan and thank you for the quick response. Much appreciated!
> >> 1) I am probably wrong, but as I see it, the current code assumes that
> >> 'CARDqGetTSFOffset' returns a little endian value.
> >> So it calls 'qwTSFOffset =  le64_to_cpu(qwTSFOffset)'. However digging in
> >> the code of 'CARDqGetTSFOffset' I don't see a reason to assume that.
> >> Which in my opinion can cuase a bad endianness cast on big endianness
> >> systems.

You're actually correct.

But the thing is that le64_to_cpu() and cpu_to_le64() do the exactly the
same thing.  If the hardware is little endian they do nothing, but if
the hardware is big endian they call bswap_64().  The only difference is
how a human reader understands it and for git annotations.

What I suspect is that we should be passing little endian data to
iowrite32().  In other words we should change the le64_to_cpu() to
cpu_to_le64().  This wouldn't change how the code works, it would just
change how it is annotated.  However, on the other hand, I see plenty of
examples where it's passing CPU endian data to iowrite32() so maybe we
should get rid of the conversion instead.  Who knows?

I suspect is that this driver has never worked on big endian CPUs...

I would say let's not change this until we're more sure what's going on.
And probably if we make changes that affect runtime as opposed to just
modifying endian annotations, then that needs to be tested.

regards,
dan carpenter


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ