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 <[email protected]>
---
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: Zhang Shurong <[email protected]>
> Sent: Wednesday, April 26, 2023 12:24 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Zhang Shurong
> <[email protected]>
> 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 <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> 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 <[email protected]>
> Sent: Wednesday, April 26, 2023 12:29 PM
> To: Zhang Shurong <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: RE: [PATCH v2 1/2] wifi: rtw88: fix incorrect error codes in rtw_debugfs_copy_from_user
>
> > -----Original Message-----
> > From: Zhang Shurong <[email protected]>
> > Sent: Wednesday, April 26, 2023 12:24 AM
> > To: [email protected]
> > Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]; Zhang Shurong
> > <[email protected]>
> > 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 <[email protected]>
>
> Reviewed-by: Ping-Ke Shih <[email protected]>
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;