On Fri, May 02, 2025 at 03:26:22PM -0700, Nicolas Bretz wrote: > On Fri, May 2, 2025 at 1:37 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > > > > On Fri, May 02, 2025 at 10:40:12AM -0700, Nicolas Bretz wrote: > > > Added one missing kfree to fsmap.c > > > > > > Signed-off-by: Nicolas Bretz <bretznic@xxxxxxxxx> > > > --- > > > fs/ext4/fsmap.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/fs/ext4/fsmap.c b/fs/ext4/fsmap.c > > > index b232c2767534..d41210abea0c 100644 > > > --- a/fs/ext4/fsmap.c > > > +++ b/fs/ext4/fsmap.c > > > @@ -304,6 +304,7 @@ static inline int ext4_getfsmap_fill(struct list_head *meta_list, > > > fsm->fmr_length = len; > > > list_add_tail(&fsm->fmr_list, meta_list); > > > > > > + kfree(fsm); > > > > OI: UAF, NAK. > > > > --D > > I apologize, it definitely wasn't my intention. I guess not really > putting my best foot forward... > I don't yet fully get the UAF in this instance, but I'm studying it. UAF == "Use After Free" What this function does is to allocate a data struture, and then add it to a linked list. This is what list_add_tail() does. By adding the kfree(), this will result in the callers of ext4_getfsap_fill() trying to dereference a pointer to memory that has been freed. So your patch very cleary introduces a bug, and makes it clear you (a) don't understand what lst_add_tail() does, and (b) dont't understand what the ext4_getfsap_fill()function does, and (c) almost certainkly didn't understand how to test a particular code path and/or didn't bother to adequately test the code that you were trying to modify. For future reference, the commit description for this patch is also not adequate. Don't replicate in English what the change in the C code is. Explain *why* the change was made; what bug were you trying to fix? Or what performance optimization were you going for? And it's often a good idea to explain how you tested your change. Cheers, - Ted
