Re: [PATCH][next] wifi: rtl8xxxu: Avoid -Wflex-array-member-not-at-end warnings

Re: [PATCH][next] wifi: rtl8xxxu: Avoid -Wflex-array-member-not-at-end warnings

[Bitterblue Smith]

On 26/11/2025 05:26, Ping-Ke Shih wrote:
> Hi Bitterblue,
> 
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> On 21/11/2025 13:11, Zenm Chen wrote:
>>> Gustavo A. R. Silva <gustavo@embeddedor.com> 於 2025年11月21日 週五 下午6:20寫道：
>>>>
>>>> Hi,
>>>>
>>>> On 11/21/25 19:06, Zenm Chen wrote:
>>>>> Dear maintainers,
>>>>>
>>>>> With this patch applied, my system always freezes right after the rtl8xxxu
>>>>> driver is loaded. is it normal?
>>>>
>>>> I don't think so... It probably means that struct urb urb; cannot really be
>>>> moved to the end of struct rtl8xxxu_rx_urb or struct rtl8xxxu_tx_urb?
>>>>
>>>> It'd be great if you could share a log.
>>>>
>>>
>>> Hi,
>>>
>>> Nothing helpful found from the kernel log. Maybe Realtek drivers maintainer
>>> Ping-Ke could take a look what is wrong next Monday.
>>>
>> [...]
>>
>> 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.
>>
> 
> 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. Could you also help to review if something
> I missed? Thanks.
> 

Sorry, I didn't find anything either. Maybe someone from linux-usb
has an idea? The errors I got are here:

https://lore.kernel.org/linux-wireless/475b4336-eed0-4fae-848f-aae26f109606@gmail.com/

> 
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1942,15 +1942,19 @@ struct rtl8xxxu_vif {
>  };
> 
>  struct rtl8xxxu_rx_urb {
> +       u8 pad[128];
>         struct urb urb;
>         struct ieee80211_hw *hw;
>         struct list_head list;
> };
> 
>  struct rtl8xxxu_tx_urb {
> +       u8 pad[128];
>         struct urb urb;
>         struct ieee80211_hw *hw;
>         struct list_head list;
> };
> 
>  struct rtl8xxxu_fileops {
> 
> 

Re: [PATCH][next] wifi: rtl8xxxu: Avoid -Wflex-array-member-not-at-end warnings

[Michal Pecio]

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).

Now a bogus address is being passed to kfree() and things go boom.
Or at least that's my first guess after spending a few minutes.
But that's the direction I would be looking at.

Regards,
Michal

Re: [PATCH][next] wifi: rtl8xxxu: Avoid -Wflex-array-member-not-at-end warnings

[Greg KH]

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.

So this needs to be fixed up no matter what.

thanks,

greg k-h

Re: [PATCH][next] wifi: rtl8xxxu: Avoid -Wflex-array-member-not-at-end warnings

[Michal Pecio]

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

Re: [PATCH][next] wifi: rtl8xxxu: Avoid -Wflex-array-member-not-at-end warnings

[Bitterblue Smith]

On 07/12/2025 01:16, 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).
> 
> Now a bogus address is being passed to kfree() and things go boom.
> Or at least that's my first guess after spending a few minutes.
> But that's the direction I would be looking at.
> 
> Regards,
> Michal

Ahhh, I see it now, thank you.