Re: [PATCH v2] dmaengine: idxd: Do not use devm for 'struct device' object allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2/24/2021 11:00 AM, Jason Gunthorpe wrote:
On Wed, Feb 24, 2021 at 09:49:39AM -0800, Dan Williams wrote:

I'd say this is follow-on work post bug-fix for the current lifetime violation.

+       kfree(idxd->msix_entries);
+       kfree(idxd->irq_entries);
+       kfree(idxd->int_handles);
These seem tied to the lifetime of the driver bind to the pci_device
not the conf_dev, or are those lines blurred here?
Jason asked those to be not allocated by devm_* so there wouldn't be any
inter-mixing of allocation methods and causing a mess.
Ok, it wasn't clear to me where the lines were drawn, so moving all to
the lowest common denominator lifetime makes sense.
Usualy when I see things like this I ask why those are dedicated
allocations too?

It looks like msix_entries is usless, this could be a temp allocation
during probe and the vector # stored in the irq_entries.

Ok.


int_handles might like to be a flex array, not sure

The other devm stuff looks questionable:

         rc = devm_request_threaded_irq(dev, msix->vector, idxd_irq_handler,
                                        idxd_misc_thread, 0, "idxd-misc",
                                        irq_entry);

So we pass irq_entry which is not devm managed memory to the irq, but
we don't remove the irq during the driver remove() function, so this
could use-after free.

However this is counter-acted by masking and synchronize_irq extra
code.

Looks to be a lot simpler/clearer to just skip devm and call release
irq instead of the other stuff.


Ok, I'll add the cleanup for that as part of the removing devm_* calls.


Jason



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux