linux-kernel - Re: [PATCH] NTB: ntb_perf: fix cast to restricted _

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: <20180119210310.GA2284@mobilestation>
Date:   Sat, 20 Jan 2018 00:03:10 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     jdmason@...zu.us, Dave Jiang <dave.jiang@...el.com>,
        Allen.Hubbe@...il.com, "Hook, Gary" <gary.hook@....com>,
        Sergey.Semin@...latforms.ru, linux-ntb@...glegroups.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] NTB: ntb_perf: fix cast to restricted __le32

On Fri, Jan 19, 2018 at 09:42:17PM +0100, Arnd Bergmann <arnd@...db.de> wrote:
> On Fri, Jan 19, 2018 at 6:30 PM, Serge Semin <fancer.lancer@...il.com> wrote:
> > Sparse is whining about the u32 and __le32 mixed usage in the
> > driver.
> >
> > drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32
> > drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
> > drivers/ntb/test/ntb_perf.c:295:37:    expected unsigned int [unsigned] [usertype] val
> > drivers/ntb/test/ntb_perf.c:295:37:    got restricted __le32 [usertype] <noident>
> > ...
> >
> > The NTB API can't be changed so ntb_spad_*() methods
> > would return either pure __le32 or __be32, since the scratchpad
> > data can have arbitrary endianness in general. In this case we
> > need to forcibly cast all the u32 to be __le32 and vise-versa
> > where it's supposed to be in accordance with the driver logic.
> >
> 
> There's got to be a better way to do this than sprinkling lots of __force
> typecasts throughout the code.
> 
> It looks like all those casts are about
> ntb_peer_spad_read()/ntb_peer_msg_write() calls, so why not change
> those function prototypes to work on __le32 types?
> 
> There should also be some form of documentation regarding why you
> need to swap the data twice, since all the ntb drivers later end up
> doing another cpu_to_le32() on the little-endian data.
> 
>        Arnd

Actually the provided patch is the best solution I could come up with.
The thing is, that the methods can't be changed. Those functions are
the part of the NTB API methods used by many drivers. So basically they
are like pci_{read,write}_config_{byte,word,dword}() methods. We can't
change their prototypes only because it's suit some driver. The methods
give an access to the NTB device dummy u32-sized registers, nothing
else. So endianness is the transmitted data settings in this case.

NTB is the technology to interconnect some two systems with possibly
different endianness (unlike PCI, which interconnect CPU with LE devices).
In this case I'd need to set some agreement up between two systems about
the endianness of the exchanged data like host and network types in
Linux networking. I've chosen the network data to be little-endian,
that's why I needed first to convert them from CPU to le32, then on
remote side convert them back from le32 to CPU.

If you have any better suggestion how the warning can be fixed, I'd
be glad to stick to it.

-Sergey