* [PATCH] wifi: rtw89: use FIELD_PREP to fill MAC quota value
@ 2022-11-03 7:24 Ping-Ke Shih
2022-11-07 12:28 ` Kalle Valo
0 siblings, 1 reply; 3+ messages in thread
From: Ping-Ke Shih @ 2022-11-03 7:24 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
Coverity reported shift 16 bits could cause sign extension and might get
an unexpected value. Since the input values are predefined and no this
kind of case, original code is safe so far. But, still changing them to
use FIELD_PREP() will be more clear and prevent mistakes in the future.
The original message of Coverity is:
Suspicious implicit sign extension: "max_cfg->cma0_dma" with type "u16"
(16 bits, unsigned) is promoted in "max_cfg->cma0_dma << 16" to type
"int" (32 bits, signed), then sign-extended to type "unsigned long"
(64 bits, unsigned). If "max_cfg->cma0_dma << 16" is greater than
0x7FFFFFFF, the upper bits of the result will all be 1."
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1527095 ("Integer handling issues")
Fixes: e3ec7017f6a2 ("rtw89: add Realtek 802.11ax driver")
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
drivers/net/wireless/realtek/rtw89/mac.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
index bb49033b587d2..11081dfdfb172 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.c
+++ b/drivers/net/wireless/realtek/rtw89/mac.c
@@ -1487,10 +1487,8 @@ static int dle_mix_cfg(struct rtw89_dev *rtwdev, const struct rtw89_dle_mem *cfg
#define INVALID_QT_WCPU U16_MAX
#define SET_QUOTA_VAL(_min_x, _max_x, _module, _idx) \
do { \
- val = ((_min_x) & \
- B_AX_ ## _module ## _MIN_SIZE_MASK) | \
- (((_max_x) << 16) & \
- B_AX_ ## _module ## _MAX_SIZE_MASK); \
+ val = FIELD_PREP(B_AX_ ## _module ## _MIN_SIZE_MASK, _min_x) | \
+ FIELD_PREP(B_AX_ ## _module ## _MAX_SIZE_MASK, _max_x); \
rtw89_write32(rtwdev, \
R_AX_ ## _module ## _QTA ## _idx ## _CFG, \
val); \
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] wifi: rtw89: use FIELD_PREP to fill MAC quota value
2022-11-03 7:24 [PATCH] wifi: rtw89: use FIELD_PREP to fill MAC quota value Ping-Ke Shih
@ 2022-11-07 12:28 ` Kalle Valo
2022-11-08 0:41 ` Ping-Ke Shih
0 siblings, 1 reply; 3+ messages in thread
From: Kalle Valo @ 2022-11-07 12:28 UTC (permalink / raw)
To: Ping-Ke Shih; +Cc: linux-wireless
Ping-Ke Shih <pkshih@realtek.com> writes:
> Coverity reported shift 16 bits could cause sign extension and might get
> an unexpected value. Since the input values are predefined and no this
> kind of case, original code is safe so far. But, still changing them to
> use FIELD_PREP() will be more clear and prevent mistakes in the future.
>
> The original message of Coverity is:
> Suspicious implicit sign extension: "max_cfg->cma0_dma" with type "u16"
> (16 bits, unsigned) is promoted in "max_cfg->cma0_dma << 16" to type
> "int" (32 bits, signed), then sign-extended to type "unsigned long"
> (64 bits, unsigned). If "max_cfg->cma0_dma << 16" is greater than
> 0x7FFFFFFF, the upper bits of the result will all be 1."
>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1527095 ("Integer handling issues")
> Fixes: e3ec7017f6a2 ("rtw89: add Realtek 802.11ax driver")
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
> drivers/net/wireless/realtek/rtw89/mac.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
> index bb49033b587d2..11081dfdfb172 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> @@ -1487,10 +1487,8 @@ static int dle_mix_cfg(struct rtw89_dev *rtwdev, const struct rtw89_dle_mem *cfg
> #define INVALID_QT_WCPU U16_MAX
> #define SET_QUOTA_VAL(_min_x, _max_x, _module, _idx) \
> do { \
> - val = ((_min_x) & \
> - B_AX_ ## _module ## _MIN_SIZE_MASK) | \
> - (((_max_x) << 16) & \
> - B_AX_ ## _module ## _MAX_SIZE_MASK); \
> + val = FIELD_PREP(B_AX_ ## _module ## _MIN_SIZE_MASK, _min_x) | \
> + FIELD_PREP(B_AX_ ## _module ## _MAX_SIZE_MASK, _max_x); \
BTW in wireless nowadays the preference is to use u32_encode_bits() & co
instead of FIELD_PREP(). Not an issue for this patch, just wanted to
mention anyway.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [PATCH] wifi: rtw89: use FIELD_PREP to fill MAC quota value
2022-11-07 12:28 ` Kalle Valo
@ 2022-11-08 0:41 ` Ping-Ke Shih
0 siblings, 0 replies; 3+ messages in thread
From: Ping-Ke Shih @ 2022-11-08 0:41 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless@vger.kernel.org
> -----Original Message-----
> From: Kalle Valo <kvalo@kernel.org>
> Sent: Monday, November 7, 2022 8:29 PM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: linux-wireless@vger.kernel.org
> Subject: Re: [PATCH] wifi: rtw89: use FIELD_PREP to fill MAC quota value
>
> Ping-Ke Shih <pkshih@realtek.com> writes:
>
> > Coverity reported shift 16 bits could cause sign extension and might get
> > an unexpected value. Since the input values are predefined and no this
> > kind of case, original code is safe so far. But, still changing them to
> > use FIELD_PREP() will be more clear and prevent mistakes in the future.
> >
> > The original message of Coverity is:
> > Suspicious implicit sign extension: "max_cfg->cma0_dma" with type "u16"
> > (16 bits, unsigned) is promoted in "max_cfg->cma0_dma << 16" to type
> > "int" (32 bits, signed), then sign-extended to type "unsigned long"
> > (64 bits, unsigned). If "max_cfg->cma0_dma << 16" is greater than
> > 0x7FFFFFFF, the upper bits of the result will all be 1."
> >
> > Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> > Addresses-Coverity-ID: 1527095 ("Integer handling issues")
> > Fixes: e3ec7017f6a2 ("rtw89: add Realtek 802.11ax driver")
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > ---
> > drivers/net/wireless/realtek/rtw89/mac.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
> > index bb49033b587d2..11081dfdfb172 100644
> > --- a/drivers/net/wireless/realtek/rtw89/mac.c
> > +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> > @@ -1487,10 +1487,8 @@ static int dle_mix_cfg(struct rtw89_dev *rtwdev, const struct rtw89_dle_mem *cfg
> > #define INVALID_QT_WCPU U16_MAX
> > #define SET_QUOTA_VAL(_min_x, _max_x, _module, _idx) \
> > do { \
> > - val = ((_min_x) & \
> > - B_AX_ ## _module ## _MIN_SIZE_MASK) | \
> > - (((_max_x) << 16) & \
> > - B_AX_ ## _module ## _MAX_SIZE_MASK); \
> > + val = FIELD_PREP(B_AX_ ## _module ## _MIN_SIZE_MASK, _min_x) | \
> > + FIELD_PREP(B_AX_ ## _module ## _MAX_SIZE_MASK, _max_x); \
>
> BTW in wireless nowadays the preference is to use u32_encode_bits() & co
> instead of FIELD_PREP(). Not an issue for this patch, just wanted to
> mention anyway.
>
I can practice to use u32_encode_bits() from this patch. Will send v2.
Ping-Ke
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-08 0:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-03 7:24 [PATCH] wifi: rtw89: use FIELD_PREP to fill MAC quota value Ping-Ke Shih
2022-11-07 12:28 ` Kalle Valo
2022-11-08 0:41 ` Ping-Ke Shih
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.