Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 12 Dec 2022 18:33:30 +0300
From:   Dan Carpenter <error27@gmail.com>
To:     pkshih@realtek.com
Cc:     linux-wireless@vger.kernel.org
Subject: [bug report] rtw89: add Realtek 802.11ax driver
Message-ID: <Y5dJymTl15fBnQtV@kili>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Precedence: bulk

Hello Ping-Ke Shih,

The patch e3ec7017f6a2: "rtw89: add Realtek 802.11ax driver" from Oct
11, 2021, leads to the following potential issue (just from reading
the code):

drivers/net/wireless/realtek/rtw89/core.h
    3878 static inline u32
    3879 rtw89_read32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
    3880 {
    3881         u32 shift = __ffs(mask);
    3882         u32 orig;
    3883         u32 ret;
    3884 
    3885         orig = rtw89_read32(rtwdev, addr);
--> 3886         ret = (orig & mask) >> shift;

I think this line should be:

	ret = (orig & mask) >> (shift - 1);

A typical mask here is 0xff so __ffs() is 1 because the first bit is
set.  This code will do: ret = (orig & 0xff) >> 1;  I think it should be
ret = (orig & 0xff) >> 0;

If the mask was 0xff00 I would expect >> 8 instead of >> 9 etc.

    3887 
    3888         return ret;
    3889 }

regards,
dan carpenter