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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 May 2020 09:43:34 +0200 From: Geert Uytterhoeven <geert@...ux-m68k.org> To: Jerome Pouiller <Jerome.Pouiller@...abs.com> Cc: driverdevel <devel@...verdev.osuosl.org>, linux-wireless <linux-wireless@...r.kernel.org>, netdev <netdev@...r.kernel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Kalle Valo <kvalo@...eaurora.org>, "David S . Miller" <davem@...emloft.net> Subject: Re: [PATCH 13/17] staging: wfx: fix endianness of the field 'len' Hi Jerome, On Mon, May 11, 2020 at 5:53 PM Jerome Pouiller <Jerome.Pouiller@...abs.com> wrote: > From: Jérôme Pouiller <jerome.pouiller@...abs.com> > > The struct hif_msg is received from the hardware. So, it declared as > little endian. However, it is also accessed from many places in the > driver. Sparse complains about that: > > drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer > drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer > drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer > drivers/staging/wfx/bh.c:93:32: warning: cast to restricted __le16 > drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer > drivers/staging/wfx/bh.c:121:25: warning: incorrect type in argument 2 (different base types) > drivers/staging/wfx/bh.c:121:25: expected unsigned int len > drivers/staging/wfx/bh.c:121:25: got restricted __le16 [usertype] len > drivers/staging/wfx/hif_rx.c:27:22: warning: restricted __le16 degrades to integer > drivers/staging/wfx/hif_rx.c:347:39: warning: incorrect type in argument 7 (different base types) > drivers/staging/wfx/hif_rx.c:347:39: expected unsigned int [usertype] len > drivers/staging/wfx/hif_rx.c:347:39: got restricted __le16 const [usertype] len > drivers/staging/wfx/hif_rx.c:365:39: warning: incorrect type in argument 7 (different base types) > drivers/staging/wfx/hif_rx.c:365:39: expected unsigned int [usertype] len > drivers/staging/wfx/hif_rx.c:365:39: got restricted __le16 const [usertype] len > drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types) > drivers/staging/wfx/./traces.h:195:1: expected int msg_len > drivers/staging/wfx/./traces.h:195:1: got restricted __le16 const [usertype] len > drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types) > drivers/staging/wfx/./traces.h:195:1: expected int msg_len > drivers/staging/wfx/./traces.h:195:1: got restricted __le16 const [usertype] len > drivers/staging/wfx/debug.c:319:20: warning: restricted __le16 degrades to integer > drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer > drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer Thanks for your patch! > In order to make Sparse happy and to keep access from the driver easy, > this patch declare 'len' with native endianness. > > On reception of hardware data, this patch takes care to do byte-swap and > keep Sparse happy. Which means sparse can no longer do any checking on the field, and new bugs may/will creep in in the future, unnoticed. > --- a/drivers/staging/wfx/hif_api_general.h > +++ b/drivers/staging/wfx/hif_api_general.h > @@ -23,7 +23,10 @@ > #define HIF_COUNTER_MAX 7 > > struct hif_msg { > - __le16 len; > + // len is in fact little endian. However, it is widely used in the > + // driver, so we declare it in native byte order and we reorder just > + // before/after send/receive it (see bh.c). > + u16 len; While there's a small penalty associated with always doing the conversion on big-endian platforms, it will probably be lost in the noise anyway. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Powered by blists - more mailing lists