[BUG] net: macb: Use-After-Free when removing the module

[BUG] net: macb: Use-After-Free when removing the module

[Zheyu Ma]

When removing the macb_pci module, the driver will cause a UAF bug.

Commit d82d5303c4c5 ("net: macb: fix use after free on rmmod") moves
the platform_device_unregister() after clk_unregister(), but this
introduces another UAF bug.

The following log reveals it:

[   64.783836] BUG: KASAN: use-after-free in clk_prepare+0x32/0x50
[   64.794805]  kasan_report+0x45/0x60
[   64.795226]  clk_prepare+0x32/0x50
[   64.795641]  macb_runtime_resume+0xc4/0x2e0
[   64.796149]  __rpm_callback+0x3e8/0xa30
[   64.796621]  ? ktime_get_mono_fast_ns+0x97/0x1c0
[   64.797172]  ? pm_generic_runtime_suspend+0xb0/0xb0
[   64.797760]  rpm_resume+0xff5/0x1860
[   64.798194]  ? _raw_spin_lock_irqsave+0x7a/0x140
[   64.798751]  __pm_runtime_resume+0x105/0x160
[   64.799253]  device_release_driver_internal+0x13c/0x7c0
[   64.799874]  bus_remove_device+0x2d0/0x340

[   64.816165] Allocated by task 252:
[   64.817167]  ____kasan_kmalloc+0xb5/0xf0
[   64.817669]  __clk_register+0x98d/0x23b0
[   64.818149]  clk_hw_register+0xb2/0xd0
[   64.818610]  clk_register_fixed_rate+0x290/0x350
[   64.819861]  macb_probe+0x233/0x590 [macb_pci]
[   64.820411]  local_pci_probe+0x13f/0x210
[   64.820890]  pci_device_probe+0x34c/0x6d0
[   64.821368]  really_probe+0x24c/0x8d0
[   64.821814]  __driver_probe_device+0x1b3/0x280
[   64.822346]  driver_probe_device+0x50/0x370

[   64.828473] Freed by task 485:
[   64.829440]  kasan_set_track+0x3d/0x70
[   64.829901]  kasan_set_free_info+0x1f/0x40
[   64.830396]  ____kasan_slab_free+0x103/0x140
[   64.830914]  kfree+0xf2/0x270
[   64.831275]  clk_unregister+0x6df/0x8d0
[   64.831739]  macb_remove+0x89/0xc0 [macb_pci]
[   64.832252]  pci_device_remove+0x92/0x240
[   64.832741]  device_release_driver_internal+0x4e4/0x7c0
[   64.833365]  driver_detach+0x1e1/0x2b0
[   64.833817]  bus_remove_driver+0xf2/0x1d0
[   64.834294]  pci_unregister_driver+0x29/0x1a0

Regards,
Zheyu Ma

Re: [BUG] net: macb: Use-After-Free when removing the module

[Jakub Kicinski]

On Thu, 3 Mar 2022 20:24:53 +0800 Zheyu Ma wrote:
> When removing the macb_pci module, the driver will cause a UAF bug.
> 
> Commit d82d5303c4c5 ("net: macb: fix use after free on rmmod") moves
> the platform_device_unregister() after clk_unregister(), but this
> introduces another UAF bug.

The layering is all weird here. macb_probe() should allocate a private
structure for the _PCI driver_ which it can then attach to 
struct pci_dev *pdev as driver data. Then free it in remove.
It shouldn't stuff its information into the platform device.

Are you willing to send a fix like that?

Re: [BUG] net: macb: Use-After-Free when removing the module

[Nicolas Ferre]

On 03/03/2022 at 16:57, Jakub Kicinski wrote:
> On Thu, 3 Mar 2022 20:24:53 +0800 Zheyu Ma wrote:
>> When removing the macb_pci module, the driver will cause a UAF bug.
>>
>> Commit d82d5303c4c5 ("net: macb: fix use after free on rmmod") moves
>> the platform_device_unregister() after clk_unregister(), but this
>> introduces another UAF bug.
> 
> The layering is all weird here. macb_probe() should allocate a private
> structure for the _PCI driver_ which it can then attach to
> struct pci_dev *pdev as driver data. Then free it in remove.
> It shouldn't stuff its information into the platform device.

The PCI file was added as an optional layer to the original "platform" 
macb driver. I think it was added to run some experiments in some test 
conditions at Cadence.

> Are you willing to send a fix like that?

I would prefer that we don't change too much the driver in the normal 
working conditions: meaning without the PCI additional glue.

my $0.02.

Regards,
   Nicolas

-- 
Nicolas Ferre

Re: [BUG] net: macb: Use-After-Free when removing the module

[Zheyu Ma]

On Thu, Mar 3, 2022 at 11:57 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 3 Mar 2022 20:24:53 +0800 Zheyu Ma wrote:
> > When removing the macb_pci module, the driver will cause a UAF bug.
> >
> > Commit d82d5303c4c5 ("net: macb: fix use after free on rmmod") moves
> > the platform_device_unregister() after clk_unregister(), but this
> > introduces another UAF bug.
>
> The layering is all weird here. macb_probe() should allocate a private
> structure for the _PCI driver_ which it can then attach to
> struct pci_dev *pdev as driver data. Then free it in remove.
> It shouldn't stuff its information into the platform device.
>
> Are you willing to send a fix like that?

Thanks for your useful suggestion, I'm willing to submit a patch.
But I'm a newbie to kernel and I think I need some time to think about
how to make such changes.

Regards,
Zheyu Ma