All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Pavel Skripkin <paskripkin@gmail.com>
Cc: Solomon Tan <wjsota@gmail.com>,
	Michael Straube <straube.linux@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Larry Finger <Larry.Finger@lwfinger.net>,
	Phillip Potter <phil@philpotter.co.uk>,
	"open list:STAGING SUBSYSTEM" <linux-staging@lists.linux.dev>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [BUG] staging: r8188eu: KASAN: slab-out-of-bounds in rtw_cmd_thread
Date: Mon, 25 Apr 2022 18:41:38 +0300	[thread overview]
Message-ID: <20220425154138.GQ2462@kadam> (raw)
In-Reply-To: <c2296090-2e9b-fafb-35da-e01b025b53b7@gmail.com>

On Sun, Apr 24, 2022 at 06:06:32PM +0300, Pavel Skripkin wrote:
> Hi Solomon,
> 
> On 4/24/22 15:11, Solomon Tan wrote:
> > On Sun, Apr 24, 2022 at 12:00:12PM +0200, Michael Straube wrote:
> > > Hi,
> > > 
> > > It looks like
> > > commit 0afaa121813e ("staging: r8188eu: use in-kernel ieee channel")
> > > intoduced a. See KASAN output below.
> > > 
> > > That commit replaced the use of struct rtw_ieee80211_channel with struct
> > > ieee80211_channel.
> > > 
> > > There are several calls to memcpy that used sizeof(struct
> > > rtw_ieee80211_channel)
> > > and now use sizeof(struct ieee80211_channel) but the sizes of these two
> > > structures are not equal.
> > > 
> > 
> > Oh no. When does this issue get triggered?
> > 
> > > regards,
> > > Michael
> > > 
> > > dmesg:
> > > 
> > >  ==================================================================
> > > [  422.214237] BUG: KASAN: slab-out-of-bounds in rtw_cmd_thread+0x1e8/0x430
> > > [r8188eu]
> > > [  422.214277] Write of size 3600 at addr ffff8881e149d200 by task
> > > RTW_CMD_THREAD/2563
> > > 
> > > [  422.214289] CPU: 11 PID: 2563 Comm: RTW_CMD_THREAD Tainted: G C OE
> > > 5.18.0-rc2-staging+ #47 94e3ca73bebf5b7fec506721475e4fff2a023bb9
> > > [  422.214301] Hardware name: Gigabyte Technology Co., Ltd. B550M S2H/B550M
> > > S2H, BIOS F15a 02/16/2022
> > > [  422.214309] Call Trace:
> > > [  422.214313]  <TASK>
> > > [  422.214317]  dump_stack_lvl+0x45/0x5b
> > > [  422.214327]  print_report.cold+0x5e/0x5dc
> > > [  422.214335]  ? kasan_set_track+0x21/0x30
> > > [  422.214342]  ? kasan_set_free_info+0x20/0x40
> > > [  422.214349]  ? rtw_cmd_thread+0x1e8/0x430 [r8188eu
> > > 91924fe1575bf49b9b37985ffde2c585d847446d]
> > > [  422.214386]  kasan_report+0xab/0x120
> > > [  422.214394]  ? rtw_cmd_thread+0x1e8/0x430 [r8188eu
> > > 91924fe1575bf49b9b37985ffde2c585d847446d]
> > > [  422.214430]  kasan_check_range+0xf6/0x1d0
> > > [  422.214436]  memcpy+0x39/0x60
> > > [  422.214442]  rtw_cmd_thread+0x1e8/0x430 [r8188eu
> > > 91924fe1575bf49b9b37985ffde2c585d847446d]
> > > [  422.214479]  ? rtw_setassocsta_cmdrsp_callback+0xd0/0xd0 [r8188eu
> > > 91924fe1575bf49b9b37985ffde2c585d847446d]
> > > [  422.214516]  kthread+0x15d/0x190
> > > [  422.214523]  ? kthread_complete_and_exit+0x20/0x20
> > > [  422.214531]  ret_from_fork+0x22/0x30
> > > [  422.214540]  </TASK>
> > 
> > Sorry, I am not familiar with KASAN. How should I interpret this output?
> > I see the paragraph above has references to rtw_cmd_thread. I assume
> > that is its way of indicating that rtw_cmd_thread is the cause of the
> > problem, but the one below refers to other functions. I'm not sure where
> > I should start looking. I would start looking at `rtw_sitesurvey_cmd` and
> > `rtw_scan_ch_decision`, which call the memcpy on the
> > rtw_ieee80211_channel structure, but they are not on the call trace.
> > 
> 
> drivers/staging/r8188eu/core/rtw_cmd.c:276: memcpy() call.
> 

What git hash are you on?  Is that the line:

	memcpy(&psurveyPara->ch[i], &ch[i], sizeof(struct ieee80211_channel));

Are you sure that's the line which crashes?  According to Smatch and my
simple grep that's dead code because "ch" is always NULL.

> As Michael said the sizes of structures do not mach and the memcpy writes
> below allocated buffer.

I feel a bit bad for not spotting this in review because I was looking
for that kind of bug.  I still don't immediately spot where the bug is.

regards,
dan carpenter

  parent reply	other threads:[~2022-04-25 15:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24 10:00 [BUG] staging: r8188eu: KASAN: slab-out-of-bounds in rtw_cmd_thread Michael Straube
2022-04-24 12:11 ` Solomon Tan
2022-04-24 15:06   ` Pavel Skripkin
2022-04-25  0:37     ` Solomon Tan
2022-04-25 15:41     ` Dan Carpenter [this message]
2022-04-26  2:48       ` Solomon Tan

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=20220425154138.GQ2462@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=paskripkin@gmail.com \
    --cc=phil@philpotter.co.uk \
    --cc=straube.linux@gmail.com \
    --cc=wjsota@gmail.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.