On 26.07.24 16:52, Steven Rostedt wrote: > On Fri, 26 Jul 2024 12:16:16 +0200 > Mathias Krause <mini...@grsecurity.net> wrote: > >>> >>> With KASAN memory checking, it would trigger a use-after-free bug. This was >>> >> >> The UAF bug is there even without KASAN. It's just that KASAN makes it >> much easier to detect and catch early. > > Well the bug happens without KASAN but the "tigger" is shown by KASAN. > I was assuming people understood that.
Ahh, okay. I'd written "report" instead of "trigger" to make it less ambiguous. >>> because the format file was not checking the file's meta data flag >>> "EVENT_FILE_FL_FREED", so it would access the event that the file meta data >>> pointed to after it was freed. >>> >>> The second bug is that the dynamic "format" file also registered a callback >>> to decrement the meta data, but the "data" pointer passed to the callback >>> was the event itself. Not the meta data to free. This would either cause a >>> memory leak (the meta data never was freed) or a crash as it could have >>> incorrectly freed the event itself. > > I need to remove the above, as I realized the release callback doesn't > get called for the "filter" but for only the "enable". That doesn't get > called until all files have no more references. So there's only one bug > here. Yeah, all files are covered by the same event_file and only "enable" does the final put. >>> >>> Link: >>> https://lore.kernel.org/all/20240719204701.1605950-1-mini...@grsecurity.net/ >>> >>> Cc: sta...@vger.kernel.org >>> Reported-by: Mathias Krause <mini...@grsecurity.net> >>> Fixes: b63db58e2fa5d ("eventfs/tracing: Add callback for release of an >>> eventfs_inode") >> >> That fixes tag looks odd as it didn't introduce the bug. It's some late >> change to v6.9 but my bisect run showed, it's triggering as early as in >> v6.6 (commit 27152bceea1d ("eventfs: Move tracing/events to eventfs")). >> >> git blame points to 5790b1fb3d67 ("eventfs: Remove eventfs_file and just >> use eventfs_inode"), which is still too young, as it's v6.7. > > But if you look at the commit I posted. It has: > > Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use > eventfs_inode") > > And you need to add that to apply this patch as it has that as the > dependency. If you try to apply this to the change that had the > original bug, it will not apply. I basically say that this patch is a > fix to the previous fix. So far so good. >> >> IMHO, this needs at least the following additional fixes tags to ensure >> all stable kernels get covered: >> >> Fixes: 5790b1fb3d67 ("eventfs: Remove eventfs_file and just use >> eventfs_inode") >> Fixes: 27152bceea1d ("eventfs: Move tracing/events to eventfs") >> >> Even if 27152bceea1d is not the real cause, just the commit making the >> bug reachable. But from looking at the history, this was always wrong? > > All stable kernels should get covered as 27152bceea1d has both a Cc > stable tag and a Fixes tag for 5790b1fb3d67. And the stable kernels > look at what commits have been backported to determine what other > commits should be backported. Now you lost me. Neither has 27152bceea1d a Cc stable tag, nor a Fixes tag for 5790b1fb3d67. It simply cannot, because it's from July 2023 (v6.6-rc1) and 5790b1fb3d67 is from October 2024 (v6.7-rc1). > By saying this fixes 27152bceea1d, it > should all work out correctly. That would be fine with me, as that's what my git bisect run pointed at as well -- the oldest commit triggering the bug. However, in your v2 it's still b63db58e2fa5d (which has a Fixes tag for 5790b1fb3d672 but not 27152bceea1d) which would suggest only kernels down to v6.7 are affected. Thanks, Mathias
