Dan Carpenter <dan.carpenter@xxxxxxxxxx> 於 2022年10月27日 週四 晚上9:59寫道: > > On Thu, Oct 27, 2022 at 09:59:46AM +0800, ChiYuan Huang wrote: > > ChiYuan Huang <u0084500@xxxxxxxxx> 於 2022年10月26日 週三 下午5:05寫道: > > > > > > Dan Carpenter <dan.carpenter@xxxxxxxxxx> 於 2022年10月26日 週三 下午4:51寫道: > > > > > > > > On Wed, Oct 26, 2022 at 03:24:48PM +0800, ChiYuan Huang wrote: > > > > > 2) normal register access with negative length > > > > > Unable to handle kernel paging request at virtual address ffffffc009cefff2 > > > > > pc : __memcpy+0x1dc/0x260 > > > > > lr : _regmap_raw_write_impl+0x6d4/0x828 > > > > > Call trace: > > > > > __memcpy+0x1dc/0x260 > > > > > _regmap_raw_write+0xb4/0x130a > > > > > regmap_raw_write+0x74/0xb0 > > > > > > > > > > > > > > > After applying the patch, the first case is cleared. > > > > > But for the case 2, the root cause is not the mt6370_regmap_write() size > > > > > check. It's in __memcpy() before mt6370_regmap_write(). > > > > > > > > > > I'm wondering 'is it reasonable to give the negative value as the size?' > > > > > > > > > > > > > Thanks for testing! > > > > > > > > I'm not sure I understand exactly which code you're talking about. > > > > Could you just create a diff with the check for negative just so I can > > > > understand where the issue is? We can re-work it into a proper patch > > > > from there. > > > > > > > Here. > > > https://elixir.bootlin.com/linux/v6.1-rc2/source/drivers/base/regmap/regmap.c#L1860 > > > > > > From my experiment, I try to access 0x00 reg for size (-1). > > > Testing code is like as below > > > regmap_raw_write(regmap, 0, &val, -1); > > > > > > That's why I think if the size check is needed, it may put into > > > regmap_raw_write() like as regmap_raw_read(). > > > > > It seems c99 already said size_t is an unsigned integer type. > > My experiment for (-1) size is not reasonable. > > (-1) means it will be converted as the UINT_MAX or ULONG_MAX. > > This will cause any unknown error like as memory violation or stack > > protection,...etc. > > > > let's check whether the negative size is reasonable or not. > > If this case dost not exist, to keep the boundary check is enough. > > I thought you were testing this from user space but it sounds like > you're doing a unit test? > Yes, with the device attribute to test regmap_raw_read() and regmap_raw_write(). I think It should be the same as the usage in kernel space. > regards, > dan carpenter >