All of lore.kernel.org
 help / color / mirror / Atom feed
From: 慕冬亮 <mudongliangabcd@gmail.com>
To: davem@davemloft.net, helmut.schaa@googlemail.com,
	kuba@kernel.org, kvalo@codeaurora.org,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	stf_xl@wp.pl, Greg KH <greg@kroah.com>
Cc: syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	syzkaller <syzkaller@googlegroups.com>
Subject: "KMSAN: uninit-value in rt2500usb_bbp_read" and "KMSAN: uninit-value in rt2500usb_probe_hw" should be duplicate crash reports
Date: Thu, 21 Jan 2021 16:47:37 +0800	[thread overview]
Message-ID: <CAD-N9QX=vVdiSf5UkuoYovamfw5a0e5RQJA0dQMOKmCbs-Gyiw@mail.gmail.com> (raw)

Dear kernel developers,

I found that on the syzbot dashboard, “KMSAN: uninit-value in
rt2500usb_bbp_read” [1] and "KMSAN: uninit-value in
rt2500usb_probe_hw" [2] should share the same root cause.

## Duplication

The reasons for the above statement:
1) The PoCs are exactly the same with each other;
2) The stack trace is almost the same except for the top 2 functions;

## Root Cause Analysis

After looking at the difference between the two stack traces, we found
they diverge at the function - rt2500usb_probe_hw.
------------------------------------------------------------------------------------------------------------------------
static int rt2500usb_probe_hw(struct rt2x00_dev *rt2x00dev)
{
        ......
        // rt2500usb_validate_eeprom->rt2500usb_bbp_read->rt2500usb_regbusy_read->rt2500usb_register_read_lock
from KMSAN
        retval = rt2500usb_validate_eeprom(rt2x00dev);
        if (retval)
                return retval;
        // rt2500usb_init_eeprom-> rt2500usb_register_read from KMSAN
        retval = rt2500usb_init_eeprom(rt2x00dev);
        if (retval)
                return retval;
------------------------------------------------------------------------------------------------------------------------
From the implementation of rt2500usb_register_read and
rt2500usb_register_read_lock, we know that, in some situation, reg is
not initialized in the function invocation
(rt2x00usb_vendor_request_buff/rt2x00usb_vendor_req_buff_lock), and
KMSAN reports uninit-value at its first memory access.
------------------------------------------------------------------------------------------------------------------------
static u16 rt2500usb_register_read(struct rt2x00_dev *rt2x00dev,
                                   const unsigned int offset)
{
        __le16 reg;
        // reg is not initialized during the following function all
        rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_READ,
                                      USB_VENDOR_REQUEST_IN, offset,
                                      &reg, sizeof(reg));
        return le16_to_cpu(reg);
}
static u16 rt2500usb_register_read_lock(struct rt2x00_dev *rt2x00dev,
                                        const unsigned int offset)
{
        __le16 reg;
        // reg is not initialized during the following function all
        rt2x00usb_vendor_req_buff_lock(rt2x00dev, USB_MULTI_READ,
                                       USB_VENDOR_REQUEST_IN, offset,
                                       &reg, sizeof(reg), REGISTER_TIMEOUT);
        return le16_to_cpu(reg);
}
------------------------------------------------------------------------------------------------------------------------
Take rt2x00usb_vendor_req_buff_lock as an example, let me illustrate
the issue when the "reg" variable is uninitialized. No matter the CSR
cache is unavailable or the status is not right, the buffer or reg
will be not initialized.
And all those issues are probabilistic events. If they occur in
rt2500usb_register_read, KMSAN reports "uninit-value in
rt2500usb_probe_hw"; Otherwise, it reports "uninit-value in
rt2500usb_bbp_read".
------------------------------------------------------------------------------------------------------------------------
int rt2x00usb_vendor_req_buff_lock(struct rt2x00_dev *rt2x00dev,
                                   const u8 request, const u8 requesttype,
                                   const u16 offset, void *buffer,
                                   const u16 buffer_length, const int timeout)
{
        if (unlikely(!rt2x00dev->csr.cache || buffer_length > CSR_CACHE_SIZE)) {
                rt2x00_err(rt2x00dev, "CSR cache not available\n");
                return -ENOMEM;
        }

        if (requesttype == USB_VENDOR_REQUEST_OUT)
                memcpy(rt2x00dev->csr.cache, buffer, buffer_length);

        status = rt2x00usb_vendor_request(rt2x00dev, request, requesttype,
                                          offset, 0, rt2x00dev->csr.cache,
                                          buffer_length, timeout);

        if (!status && requesttype == USB_VENDOR_REQUEST_IN)
                memcpy(buffer, rt2x00dev->csr.cache, buffer_length);

        return status;
}
------------------------------------------------------------------------------------------------------------------------

## Patch

I propose to memset reg variable before invoking
rt2x00usb_vendor_req_buff_lock/rt2x00usb_vendor_request_buff.

------------------------------------------------------------------------------------------------------------------------
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
index fce05fc88aaf..f6c93a25b18c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
@@ -48,6 +48,7 @@ static u16 rt2500usb_register_read(struct rt2x00_dev
*rt2x00dev,
                                   const unsigned int offset)
 {
        __le16 reg;
+       memset(&reg, 0, sizeof(reg));
        rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_READ,
                                      USB_VENDOR_REQUEST_IN, offset,
                                      &reg, sizeof(reg));
@@ -58,6 +59,7 @@ static u16 rt2500usb_register_read_lock(struct
rt2x00_dev *rt2x00dev,
                                        const unsigned int offset)
 {
        __le16 reg;
+       memset(&reg, 0, sizeof(reg));
        rt2x00usb_vendor_req_buff_lock(rt2x00dev, USB_MULTI_READ,
                                       USB_VENDOR_REQUEST_IN, offset,
                                       &reg, sizeof(reg), REGISTER_TIMEOUT);
------------------------------------------------------------------------------------------------------------------------

If you can have any issues with this statement or our information is
useful to you, please let us know. Thanks very much.

[1] “KMSAN: uninit-value in rt2500usb_bbp_read” -
https://syzkaller.appspot.com/bug?id=f35d123de7d393019c1ed4d4e60dc66596ed62cd
[2] “KMSAN: uninit-value in rt2500usb_probe_hw” -
https://syzkaller.appspot.com/bug?id=5402df7259c74e15a12992e739b5ac54c9b8a4ce


--
My best regards to you.

     No System Is Safe!
     Dongliang Mu

             reply	other threads:[~2021-01-21 19:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21  8:47 慕冬亮 [this message]
2021-01-21  8:52 ` "KMSAN: uninit-value in rt2500usb_bbp_read" and "KMSAN: uninit-value in rt2500usb_probe_hw" should be duplicate crash reports Greg KH
2021-01-21  9:24   ` 慕冬亮
2021-01-21  9:25 ` Stanislaw Gruszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAD-N9QX=vVdiSf5UkuoYovamfw5a0e5RQJA0dQMOKmCbs-Gyiw@mail.gmail.com' \
    --to=mudongliangabcd@gmail.com \
    --cc=davem@davemloft.net \
    --cc=greg@kroah.com \
    --cc=helmut.schaa@googlemail.com \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stf_xl@wp.pl \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=syzkaller@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.