| Message ID | tencent_8BA35E4B9CDF40D4AE6D8D831D7ACC28A00A@qq.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: rtw88: error codes fix patches | expand |
> -----Original Message----- > From: Zhang Shurong <zhang_shurong@foxmail.com> > Sent: Wednesday, April 26, 2023 12:24 AM > 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 v2 1/2] wifi: rtw88: fix incorrect error codes in rtw_debugfs_copy_from_user > > If there is a failure during copy_from_user, rtw_debugfs_copy_from_user > 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> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com> > --- > drivers/net/wireless/realtek/rtw88/debug.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c > index fa3d73b333ba..3da477e1ebd3 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; > > tmp[tmp_len] = '\0'; > > -- > 2.40.0
> -----Original Message----- > From: Ping-Ke Shih <pkshih@realtek.com> > Sent: Wednesday, April 26, 2023 12:29 PM > To: Zhang Shurong <zhang_shurong@foxmail.com>; 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 > Subject: RE: [PATCH v2 1/2] wifi: rtw88: fix incorrect error codes in rtw_debugfs_copy_from_user > > > -----Original Message----- > > From: Zhang Shurong <zhang_shurong@foxmail.com> > > Sent: Wednesday, April 26, 2023 12:24 AM > > 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 v2 1/2] wifi: rtw88: fix incorrect error codes in rtw_debugfs_copy_from_user > > > > If there is a failure during copy_from_user, rtw_debugfs_copy_from_user > > 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> > > Reviewed-by: Ping-Ke Shih <pkshih@realtek.com> I would take back this temporarily because of below. > > > --- > > drivers/net/wireless/realtek/rtw88/debug.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c > > index fa3d73b333ba..3da477e1ebd3 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; > > > > tmp[tmp_len] = '\0'; > > In the second patch, you check 'ret < 0' instead of 'ret'. That looks like you can possibly return positive value (e.g. count), but actually only return 0 or - EFAULT after this patch. So, I would like change first or second patch to make them intuitive. return 0 or -EFAULT --> check by if (ret) return 0 or -EFAULT or count --> check by if (ret < 0) + ret = rtw_debugfs_copy_from_user(tmp, sizeof(tmp), buffer, count, 2); + if (ret < 0) + return ret;
diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c index fa3d73b333ba..3da477e1ebd3 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; tmp[tmp_len] = '\0';
If there is a failure during copy_from_user, rtw_debugfs_copy_from_user 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)