| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 24 Apr 2023 14:28:35 +0800
From: foxmail <zhang_shurong@...mail.com>
To: Ping-Ke Shih <pkshih@...ltek.com>
Cc: "tony0620emma@...il.com" <tony0620emma@...il.com>,
"kvalo@...nel.org" <kvalo@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 01/10] wifi: rtw88: fix incorrect error codes in
rtw_debugfs_set_write_reg
Thank you a lot for your kind reply, I will resend it as 2 patches.
> 2023年4月24日 09:58，Ping-Ke Shih <pkshih@...ltek.com> 写道：
>
>> -----Original Message-----
>> From: Zhang Shurong <zhang_shurong@...mail.com>
>> Sent: Saturday, April 22, 2023 6:05 PM
>> To: tony0620emma@...il.com
>> Cc: kvalo@...nel.org; davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
>> linux-wireless@...r.kernel.org; netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Zhang Shurong
>> <zhang_shurong@...mail.com>
>> Subject: [PATCH 01/10] wifi: rtw88: fix incorrect error codes in rtw_debugfs_set_write_reg
>>
>> If there is a failure during copy_from_user or user-provided data
>> buffer is invalid, rtw_debugfs_set_write_reg should return negative
>> error code instead of a positive value count.
>>
>> Fix this bug by returning correct error code. Moreover, the check
>> of buffer against null is removed since it will be handled by
>> copy_from_user.
>>
>> Signed-off-by: Zhang Shurong <zhang_shurong@...mail.com>
>> ---
>> drivers/net/wireless/realtek/rtw88/debug.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c
>> index fa3d73b333ba..bc41c5a7acaf 100644
>> --- a/drivers/net/wireless/realtek/rtw88/debug.c
>> +++ b/drivers/net/wireless/realtek/rtw88/debug.c
>> @@ -183,8 +183,8 @@ static int rtw_debugfs_copy_from_user(char tmp[], int size,
>>
>> tmp_len = (count > size - 1 ? size - 1 : count);
>>
>> - if (!buffer || copy_from_user(tmp, buffer, tmp_len))
>> - return count;
>> + if (copy_from_user(tmp, buffer, tmp_len))
>> + return -EFAULT;
>
> This patchset is fine to me. The only thing is this chunk can be first patch,
> and squash other patches to second patch because they do the same thing
> in the same driver.
>
>
>>
>> tmp[tmp_len] = '\0';
>>
>> @@ -338,14 +338,17 @@ static ssize_t rtw_debugfs_set_write_reg(struct file *filp,
>> char tmp[32 + 1];
>> u32 addr, val, len;
>> int num;
>> + int ret;
>>
>> - rtw_debugfs_copy_from_user(tmp, sizeof(tmp), buffer, count, 3);
>> + ret = rtw_debugfs_copy_from_user(tmp, sizeof(tmp), buffer, count, 3);
>> + if (ret < 0)
>> + return ret;
>>
>> /* write BB/MAC register */
>> num = sscanf(tmp, "%x %x %x", &addr, &val, &len);
>>
>> if (num != 3)
>> - return count;
>> + return -EINVAL;
>>
>> switch (len) {
>> case 1:
>> --
>> 2.40.0
>
>> -----Original Message-----
>> From: Zhang Shurong <zhang_shurong@...mail.com>
>> Sent: Saturday, April 22, 2023 6:05 PM
>> To: tony0620emma@...il.com
>> Cc: kvalo@...nel.org; davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
>> linux-wireless@...r.kernel.org; netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Zhang Shurong
>> <zhang_shurong@...mail.com>
>> Subject: [PATCH 01/10] wifi: rtw88: fix incorrect error codes in rtw_debugfs_set_write_reg
>>
>> If there is a failure during copy_from_user or user-provided data
>> buffer is invalid, rtw_debugfs_set_write_reg should return negative
>> error code instead of a positive value count.
>>
>> Fix this bug by returning correct error code. Moreover, the check
>> of buffer against null is removed since it will be handled by
>> copy_from_user.
>>
>> Signed-off-by: Zhang Shurong <zhang_shurong@...mail.com>
>> ---
>> drivers/net/wireless/realtek/rtw88/debug.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c
>> index fa3d73b333ba..bc41c5a7acaf 100644
>> --- a/drivers/net/wireless/realtek/rtw88/debug.c
>> +++ b/drivers/net/wireless/realtek/rtw88/debug.c
>> @@ -183,8 +183,8 @@ static int rtw_debugfs_copy_from_user(char tmp[], int size,
>>
>> tmp_len = (count > size - 1 ? size - 1 : count);
>>
>> - if (!buffer || copy_from_user(tmp, buffer, tmp_len))
>> - return count;
>> + if (copy_from_user(tmp, buffer, tmp_len))
>> + return -EFAULT;
>
> This patchset is fine to me. The only thing is this chunk can be first patch,
> and squash other patches to second patch because they do the same thing
> in the same driver.
>
>
>>
>> tmp[tmp_len] = '\0';
>>
>> @@ -338,14 +338,17 @@ static ssize_t rtw_debugfs_set_write_reg(struct file *filp,
>> char tmp[32 + 1];
>> u32 addr, val, len;
>> int num;
>> + int ret;
>>
>> - rtw_debugfs_copy_from_user(tmp, sizeof(tmp), buffer, count, 3);
>> + ret = rtw_debugfs_copy_from_user(tmp, sizeof(tmp), buffer, count, 3);
>> + if (ret < 0)
>> + return ret;
>>
>> /* write BB/MAC register */
>> num = sscanf(tmp, "%x %x %x", &addr, &val, &len);
>>
>> if (num != 3)
>> - return count;
>> + return -EINVAL;
>>
>> switch (len) {
>> case 1:
>> --
>> 2.40.0
Powered by blists - more mailing lists