| Message ID | 20221108013858.10806-1-pkshih@realtek.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 525c06c81d75690a9b795cc62a758838c1a6b6fe |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] wifi: rtw89: use u32_encode_bits() to fill MAC quota value | expand |
> -----Original Message----- > From: Ping-Ke Shih <pkshih@realtek.com> > Sent: Tuesday, November 8, 2022 9:39 AM > To: kvalo@kernel.org > Cc: linux-wireless@vger.kernel.org > Subject: [PATCH v2] wifi: rtw89: use u32_encode_bits() to fill MAC quota value > > 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 u32_encode_bits() 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> Sorry, I forget to add changelog of v2. v2: use u32_encode_bits() instead of FIELD_PREP() to set field value. > --- > 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..814ca4bc22587 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 = u32_encode_bits(_min_x, B_AX_ ## _module ## _MIN_SIZE_MASK) | \ > + u32_encode_bits(_max_x, B_AX_ ## _module ## _MAX_SIZE_MASK); \ > rtw89_write32(rtwdev, \ > R_AX_ ## _module ## _QTA ## _idx ## _CFG, \ > val); \ > -- > 2.25.1 > > > ------Please consider the environment before printing this e-mail.
Ping-Ke Shih <pkshih@realtek.com> wrote: > 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 u32_encode_bits() 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> Patch applied to wireless-next.git, thanks. 525c06c81d75 wifi: rtw89: use u32_encode_bits() to fill MAC quota value
diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index bb49033b587d2..814ca4bc22587 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 = u32_encode_bits(_min_x, B_AX_ ## _module ## _MIN_SIZE_MASK) | \ + u32_encode_bits(_max_x, B_AX_ ## _module ## _MAX_SIZE_MASK); \ rtw89_write32(rtwdev, \ R_AX_ ## _module ## _QTA ## _idx ## _CFG, \ val); \
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 u32_encode_bits() 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(-)