[PATCH rtw-next] wifi: rtw89: debug: return the error when a read handler fails

[PATCH rtw-next] wifi: rtw89: debug: return the error when a read handler fails

[Devin Wittmayer]

I hit this while testing an RTL8922AU patch. Reading read_reg in debugfs
before choosing a register set off a kernel BUG that killed the reader.
It does the same every time on an RTL8852BU:

  rtw89_8852bu 2-1:1.0: invalid read reg len 0
  usercopy: Kernel memory exposure attempt detected from SLUB object 'kmalloc-8k' (offset 56, size 262144)!
  kernel BUG at mm/usercopy.c:102!

After that the driver cannot be unloaded, and on my test box network
configuration hung until a power cycle. Without hardened usercopy the
reads return kernel memory instead, and cat kept reading until I
stopped it.

The read path passes the handler's error to the copy as a length, and
saves it for reads at a later offset. Return the error before the copy.

  RTL8852BU over USB                 before    after
  read_reg, nothing chosen
    6.18.52                          BUG       -EINVAL
    7.2.6                            BUG       -EINVAL
    6.18.52, hardened usercopy off   256 KB    -EINVAL
  mac_mem_dump at offset 1,
  after a failed read
    6.18.52                          BUG       -ENOENT
    7.2.6                            BUG       -ENOENT
    6.18.52, hardened usercopy off   256 KB    -ENOENT

  RTL8922AU over USB                 before    after
  read_reg, nothing chosen
    7.2.6                            BUG       -EINVAL
  mac_mem_dump at offset 1,
  after a failed read
    7.2.6                            BUG       -ENOENT

Fixes: 831cceed3baf ("wifi: rtw89: debugfs: implement file_ops::read/write to replace seq_file")
Cc: stable@vger.kernel.org
Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>
---
 drivers/net/wireless/realtek/rtw89/debug.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw89/debug.c b/drivers/net/wireless/realtek/rtw89/debug.c
index 3a430bd08..3b7f8a809 100644
--- a/drivers/net/wireless/realtek/rtw89/debug.c
+++ b/drivers/net/wireless/realtek/rtw89/debug.c
@@ -185,6 +185,9 @@ static ssize_t rtw89_debugfs_file_read(struct file *file, char __user *userbuf,
 	debugfs_priv->rused = n;
 
 out:
+	if (n < 0)
+		return n;
+
 	return simple_read_from_buffer(userbuf, count, ppos, buf, n);
 }
 
-- 
2.56.0

RE: [PATCH rtw-next] wifi: rtw89: debug: return the error when a read handler fails

[Ping-Ke Shih]

Devin Wittmayer <lucid_duck@justthetip.ca> wrote:
> I hit this while testing an RTL8922AU patch. Reading read_reg in debugfs
> before choosing a register set off a kernel BUG that killed the reader.
> It does the same every time on an RTL8852BU:
> 
>   rtw89_8852bu 2-1:1.0: invalid read reg len 0
>   usercopy: Kernel memory exposure attempt detected from SLUB object 'kmalloc-8k' (offset 56, size
> 262144)!
>   kernel BUG at mm/usercopy.c:102!

The change of code is fine, but please reword commit message. 
I think only need this kernel log, and single sentence like
subject says. For others, you can put them after your s-o-b
following --- delimiter. 

> 
> After that the driver cannot be unloaded, and on my test box network
> configuration hung until a power cycle. Without hardened usercopy the
> reads return kernel memory instead, and cat kept reading until I
> stopped it.
> 
> The read path passes the handler's error to the copy as a length, and
> saves it for reads at a later offset. Return the error before the copy.
> 
>   RTL8852BU over USB                 before    after
>   read_reg, nothing chosen
>     6.18.52                          BUG       -EINVAL
>     7.2.6                            BUG       -EINVAL
>     6.18.52, hardened usercopy off   256 KB    -EINVAL
>   mac_mem_dump at offset 1,
>   after a failed read
>     6.18.52                          BUG       -ENOENT
>     7.2.6                            BUG       -ENOENT
>     6.18.52, hardened usercopy off   256 KB    -ENOENT
> 
>   RTL8922AU over USB                 before    after
>   read_reg, nothing chosen
>     7.2.6                            BUG       -EINVAL
>   mac_mem_dump at offset 1,
>   after a failed read
>     7.2.6                            BUG       -ENOENT
> 
> Fixes: 831cceed3baf ("wifi: rtw89: debugfs: implement file_ops::read/write to replace seq_file")
> Cc: stable@vger.kernel.org
> Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>
---

(I mean here.)

> ---
>  drivers/net/wireless/realtek/rtw89/debug.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/debug.c
> b/drivers/net/wireless/realtek/rtw89/debug.c
> index 3a430bd08..3b7f8a809 100644
> --- a/drivers/net/wireless/realtek/rtw89/debug.c
> +++ b/drivers/net/wireless/realtek/rtw89/debug.c
> @@ -185,6 +185,9 @@ static ssize_t rtw89_debugfs_file_read(struct file *file, char __user *userbuf,
>         debugfs_priv->rused = n;
> 
>  out:
> +       if (n < 0)
> +               return n;
> +
>         return simple_read_from_buffer(userbuf, count, ppos, buf, n);
>  }
> 
> --
> 2.56.0