Messages in this thread | | | Subject | Re: [PATCH 01/10] wifi: rtw88: fix incorrect error codes in rtw_debugfs_set_write_reg | From | foxmail <> | Date | Mon, 24 Apr 2023 14:28:35 +0800 |
| |
Thank you a lot for your kind reply, I will resend it as 2 patches.
> 2023年4月24日 09:58,Ping-Ke Shih <pkshih@realtek.com> 写道: > >> -----Original Message----- >> From: Zhang Shurong <zhang_shurong@foxmail.com> >> Sent: Saturday, April 22, 2023 6:05 PM >> To: tony0620emma@gmail.com >> Cc: kvalo@kernel.org; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; >> linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Zhang Shurong >> <zhang_shurong@foxmail.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@foxmail.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@foxmail.com> >> Sent: Saturday, April 22, 2023 6:05 PM >> To: tony0620emma@gmail.com >> Cc: kvalo@kernel.org; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; >> linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Zhang Shurong >> <zhang_shurong@foxmail.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@foxmail.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
| |