* [PATCH] rsi: fix integer overflow warning
@ 2017-10-05 12:05 Arnd Bergmann
2017-10-05 12:19 ` Joe Perches
[not found] ` <20171005120547.328687-1-arnd-r2nGTMty4D4@public.gmane.org>
0 siblings, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2017-10-05 12:05 UTC (permalink / raw)
To: Kalle Valo, Prameela Rani Garnepudi, Amitkumar Karwar
Cc: Arnd Bergmann, Pavani Muthyala, Karun Eagalapati, linux-wireless,
netdev, linux-kernel
gcc produces a harmless warning about a recently introduced
signed integer overflow:
drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
include/uapi/linux/swab.h:13:15: error: integer overflow in expression [-Werror=overflow]
(((__u16)(x) & (__u16)0x00ffU) << 8) | \
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
include/uapi/linux/swab.h:104:2: note: in expansion of macro '___constant_swab16'
___constant_swab16(x) : \
^~~~~~~~~~~~~~~~~~
include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
#define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
^~~~~~~~
include/linux/byteorder/generic.h:89:21: note: in expansion of macro '__cpu_to_le16'
#define cpu_to_le16 __cpu_to_le16
^~~~~~~~~~~~~
drivers/net/wireless/rsi/rsi_91x_hal.c:136:3: note: in expansion of macro 'cpu_to_le16'
cpu_to_le16((tx_params->vap_id << RSI_DESC_VAP_ID_OFST) &
^~~~~~~~~~~
The problem is that the 'mask' value is a signed integer that gets
turned into a negative number when truncated to 16 bits. Making it
an unsigned constant avoids this.
Fixes: eac4eed3224b ("rsi: tx and rx path enhancements for p2p mode")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/rsi/rsi_mgmt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_mgmt.h b/drivers/net/wireless/rsi/rsi_mgmt.h
index b9d0802c1b0f..e21723013f8d 100644
--- a/drivers/net/wireless/rsi/rsi_mgmt.h
+++ b/drivers/net/wireless/rsi/rsi_mgmt.h
@@ -189,7 +189,7 @@
IEEE80211_WMM_IE_STA_QOSINFO_AC_BE | \
IEEE80211_WMM_IE_STA_QOSINFO_AC_BK)
-#define RSI_DESC_VAP_ID_MASK 0xC000
+#define RSI_DESC_VAP_ID_MASK 0xC000u
#define RSI_DESC_VAP_ID_OFST 14
#define RSI_DATA_DESC_MAC_BBP_INFO BIT(0)
#define RSI_DATA_DESC_NO_ACK_IND BIT(9)
--
2.9.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] rsi: fix integer overflow warning
2017-10-05 12:05 [PATCH] rsi: fix integer overflow warning Arnd Bergmann
@ 2017-10-05 12:19 ` Joe Perches
2017-10-05 15:12 ` David Laight
[not found] ` <20171005120547.328687-1-arnd-r2nGTMty4D4@public.gmane.org>
1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2017-10-05 12:19 UTC (permalink / raw)
To: Arnd Bergmann, Kalle Valo, Prameela Rani Garnepudi,
Amitkumar Karwar
Cc: Pavani Muthyala, Karun Eagalapati, linux-wireless, netdev,
linux-kernel
On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> gcc produces a harmless warning about a recently introduced
> signed integer overflow:
>
> drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> include/uapi/linux/swab.h:13:15: error: integer overflow in expression [-Werror=overflow]
> (((__u16)(x) & (__u16)0x00ffU) << 8) | \
> ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> include/uapi/linux/swab.h:104:2: note: in expansion of macro '___constant_swab16'
> ___constant_swab16(x) : \
> ^~~~~~~~~~~~~~~~~~
> include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
> #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
[]
> The problem is that the 'mask' value is a signed integer that gets
> turned into a negative number when truncated to 16 bits. Making it
> an unsigned constant avoids this.
I would expect there are more of these.
Perhaps this define in include/uapi/linux/swab.h:
#define __swab16(x) \
(__builtin_constant_p((__u16)(x)) ? \
___constant_swab16(x) : \
__fswab16(x))
should be
#define __swab16(x) \
(__builtin_c
onstant_p((__u16)(x)) ? \
___constant_swab16((__u16)(x)) :
\
__fswab16((__u16)(x)))
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] rsi: fix integer overflow warning
2017-10-05 12:19 ` Joe Perches
@ 2017-10-05 15:12 ` David Laight
2017-10-05 16:11 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: David Laight @ 2017-10-05 15:12 UTC (permalink / raw)
To: 'Joe Perches', Arnd Bergmann, Kalle Valo,
Prameela Rani Garnepudi, Amitkumar Karwar
Cc: Pavani Muthyala, Karun Eagalapati, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
From: Joe Perches
> Sent: 05 October 2017 13:19
> On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> > gcc produces a harmless warning about a recently introduced
> > signed integer overflow:
> >
> > drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> > include/uapi/linux/swab.h:13:15: error: integer overflow in expression [-Werror=overflow]
> > (((__u16)(x) & (__u16)0x00ffU) << 8) | \
> > ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> > include/uapi/linux/swab.h:104:2: note: in expansion of macro '___constant_swab16'
> > ___constant_swab16(x) : \
> > ^~~~~~~~~~~~~~~~~~
> > include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
> > #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
>
> []
>
> > The problem is that the 'mask' value is a signed integer that gets
> > turned into a negative number when truncated to 16 bits. Making it
> > an unsigned constant avoids this.
>
> I would expect there are more of these.
>
> Perhaps this define in include/uapi/linux/swab.h:
>
> #define __swab16(x) \
> (__builtin_constant_p((__u16)(x)) ? \
> ___constant_swab16(x) : \
> __fswab16(x))
>
> should be
>
> #define __swab16(x) \
> (__builtin_constant_p((__u16)(x)) ? \
> ___constant_swab16((__u16)(x)) : \
> __fswab16((__u16)(x)))
You probably don't want the cast in the call to __fswab16() since
that is likely to generate an explicit and with 0xffff.
You will likely also get one if the argument is _u16 (not unsigned int).
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rsi: fix integer overflow warning
2017-10-05 15:12 ` David Laight
@ 2017-10-05 16:11 ` Joe Perches
0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2017-10-05 16:11 UTC (permalink / raw)
To: David Laight, Arnd Bergmann, Kalle Valo, Prameela Rani Garnepudi,
Amitkumar Karwar
Cc: Pavani Muthyala, Karun Eagalapati, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
On Thu, 2017-10-05 at 15:12 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 05 October 2017 13:19
> > On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> > > gcc produces a harmless warning about a recently introduced
> > > signed integer overflow:
> > >
> > > drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> > > include/uapi/linux/swab.h:13:15: error: integer overflow in expression [-Werror=overflow]
> > > (((__u16)(x) & (__u16)0x00ffU) << 8) | \
> > > ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> > > include/uapi/linux/swab.h:104:2: note: in expansion of macro '___constant_swab16'
> > > ___constant_swab16(x) : \
> > > ^~~~~~~~~~~~~~~~~~
> > > include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
> > > #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
> >
> > []
> >
> > > The problem is that the 'mask' value is a signed integer that gets
> > > turned into a negative number when truncated to 16 bits. Making it
> > > an unsigned constant avoids this.
> >
> > I would expect there are more of these.
> >
> > Perhaps this define in include/uapi/linux/swab.h:
> >
> > #define __swab16(x) \
> > (__builtin_constant_p((__u16)(x)) ? \
> > ___constant_swab16(x) : \
> > __fswab16(x))
> >
> > should be
> >
> > #define __swab16(x) \
> > (__builtin_constant_p((__u16)(x)) ? \
> > ___constant_swab16((__u16)(x)) : \
> > __fswab16((__u16)(x)))
>
> You probably don't want the cast in the call to __fswab16() since
> that is likely to generate an explicit and with 0xffff.
> You will likely also get one if the argument is _u16 (not unsigned int).
It would just an explicit vs implicit cast as __fswab16 is
a static inline with a __u16 argument
^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20171005120547.328687-1-arnd-r2nGTMty4D4@public.gmane.org>]
* Re: rsi: fix integer overflow warning
[not found] ` <20171005120547.328687-1-arnd-r2nGTMty4D4@public.gmane.org>
@ 2017-10-13 10:00 ` Kalle Valo
0 siblings, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2017-10-13 10:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Prameela Rani Garnepudi, Amitkumar Karwar, Arnd Bergmann,
Pavani Muthyala, Karun Eagalapati,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> gcc produces a harmless warning about a recently introduced
> signed integer overflow:
>
> drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> include/uapi/linux/swab.h:13:15: error: integer overflow in expression [-Werror=overflow]
> (((__u16)(x) & (__u16)0x00ffU) << 8) | \
> ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> include/uapi/linux/swab.h:104:2: note: in expansion of macro '___constant_swab16'
> ___constant_swab16(x) : \
> ^~~~~~~~~~~~~~~~~~
> include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
> #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
> ^~~~~~~~
> include/linux/byteorder/generic.h:89:21: note: in expansion of macro '__cpu_to_le16'
> #define cpu_to_le16 __cpu_to_le16
> ^~~~~~~~~~~~~
> drivers/net/wireless/rsi/rsi_91x_hal.c:136:3: note: in expansion of macro 'cpu_to_le16'
> cpu_to_le16((tx_params->vap_id << RSI_DESC_VAP_ID_OFST) &
> ^~~~~~~~~~~
>
> The problem is that the 'mask' value is a signed integer that gets
> turned into a negative number when truncated to 16 bits. Making it
> an unsigned constant avoids this.
>
> Fixes: eac4eed3224b ("rsi: tx and rx path enhancements for p2p mode")
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Patch applied to wireless-drivers-next.git, thanks.
a39644b235c1 rsi: fix integer overflow warning
--
https://patchwork.kernel.org/patch/9986961/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-13 10:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-05 12:05 [PATCH] rsi: fix integer overflow warning Arnd Bergmann
2017-10-05 12:19 ` Joe Perches
2017-10-05 15:12 ` David Laight
2017-10-05 16:11 ` Joe Perches
[not found] ` <20171005120547.328687-1-arnd-r2nGTMty4D4@public.gmane.org>
2017-10-13 10:00 ` Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).