[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251207090501.41980a50.michal.pecio@gmail.com>
Date: Sun, 7 Dec 2025 09:05:01 +0100
From: Michal Pecio <michal.pecio@...il.com>
To: Greg KH <greg@...ah.com>
Cc: Bitterblue Smith <rtl8821cerfe2@...il.com>, Ping-Ke Shih
<pkshih@...ltek.com>, Zenm Chen <zenmchen@...il.com>,
"gustavo@...eddedor.com" <gustavo@...eddedor.com>, "Jes.Sorensen@...il.com"
<Jes.Sorensen@...il.com>, "gustavoars@...nel.org" <gustavoars@...nel.org>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
linux-usb@...r.kernel.org
Subject: Re: [PATCH][next] wifi: rtl8xxxu: Avoid
-Wflex-array-member-not-at-end warnings
On Sun, 7 Dec 2025 08:55:59 +0900, Greg KH wrote:
> On Sun, Dec 07, 2025 at 12:16:08AM +0100, Michal Pecio wrote:
> > Hi,
> >
> > > >> I got something. In my case everything seemed fine until I
> > > >> unplugged the wifi adapter. And then the system still worked
> > > >> for a few minutes before it froze.
> >
> > Sounds like memory corruption.
> >
> > > > Zenm and I tested below changes which can also reproduce the
> > > > symptom, so I wonder driver might assume urb is the first
> > > > member of struct, but unfortunately I can't find that.
> >
> > That's what it seems to be doing, because it uses usb_init_urb()
> > on urbs embedded in some struct and then usb_free_urb().
> >
> > If you look what usb_free_urb() does, it decrements refcount and
> > attempts to free urb. But here urb is a member of a larger struct,
> > so I guess the whole struct is freed (and this was either
> > intentional or a bug that didn't happen to blow up yet).
>
> That's not ok at all, it's amazing this is working today. urbs need
> to be "stand alone" structures and never embedded into anything else.
Is it though?
usb_init_urb() is exported and documented as below. Neither of which
suggests that the function must not be used by drivers.
/**
* usb_init_urb - initializes a urb so that it can be used by a USB driver
* @urb: pointer to the urb to initialize
*
* Initializes a urb so that the USB subsystem can use it properly.
*
* If a urb is created with a call to usb_alloc_urb() it is not
* necessary to call this function. Only use this if you allocate the
* space for a struct urb on your own. If you call this function, be
* careful when freeing the memory for your urb that it is no longer in
* use by the USB core.
*
* Only use this function if you _really_ understand what you are doing.
*/
I see that there are some sound drivers which embed URBs in larger
structures too, so if this is some tree-wide campaign there is a risk
of breaking them too.
Regards,
Michal
Powered by blists - more mailing lists