Messages in this thread | | | From | Andy Shevchenko <> | Date | Sun, 25 Jul 2021 00:02:32 +0300 | Subject | Re: 5.14-rc failure to resume |
| |
On Sat, Jul 24, 2021 at 11:48 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sat, Jul 24, 2021 at 12:44 PM Jens Axboe <axboe@kernel.dk> wrote: > > > > This does appear to be the culprit. With it reverted on top of current > > master (and with the block and io_uring changes pulled in too), the > > kernel survives many resumes without issue. > > That commit seems fundamentally buggy. > > It makes "acpi_dev_get_next_match_dev()" always do > > acpi_dev_put(adev); > > to put the previous device, but "adev" is perfectly valid as NULL, and > acpi_dev_get_next_match_dev() even tests for it: > > struct device *start = adev ? &adev->dev : NULL; > > so it can - and will - do > > acpi_dev_put(NULL); > > which does > > put_device(&adev->dev); > > and passes in an invalid pointer to put_device(). > > And yes, that adev very much can be NULL, with drivers/acpi/utils.c > even passing it in explicitly: > > struct acpi_device * > acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv) > { > return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv); > } > EXPORT_SYMBOL(acpi_dev_get_first_match_dev); > > Am I missing something? How does that code work at all for anybody? > > I probably _am_ missing something.
Yeah, the two changes (acpi_dev_put() and this fix) were in separate submissions during different times. So, what we have here: 1) this fix misses that one line to be changed and after adding it 2) reveals UAF, or i.o.w. NULL dereference which is a bug in acpi_dev_put().
acpi_dev_put() has to be
if (adev) put_device(&adev->dev);
-- With Best Regards, Andy Shevchenko
| |