| Message ID | 20240416123427.614899-2-aalbersh@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfsprogs random fixes found by Coverity scan | expand |
On Tue, Apr 16, 2024 at 02:34:23PM +0200, Andrey Albershteyn wrote: > When count is zero fl reference is lost. Fix it by freeing the list. > > Fixes: a0d79cb37a36 ("xfs_db: make flist_find_ftyp() to check for field existance on disk") > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> Yep, that's a leak. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > db/flist.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/db/flist.c b/db/flist.c > index c81d229ab99c..0a6cc5fcee43 100644 > --- a/db/flist.c > +++ b/db/flist.c > @@ -424,8 +424,10 @@ flist_find_ftyp( > if (f->ftyp == type) > return fl; > count = fcount(f, obj, startoff); > - if (!count) > + if (!count) { > + flist_free(fl); > continue; > + } > fa = &ftattrtab[f->ftyp]; > if (fa->subfld) { > flist_t *nfl; > -- > 2.42.0 > >
On Tue, Apr 16, 2024 at 02:34:23PM +0200, Andrey Albershteyn wrote: > + if (!count) { > + flist_free(fl); > continue; > + } This looks good. The more obvious way would be move the whole loop body into a helper with two clear exits, one that returns fl, and one that frees it and returns NULL..
diff --git a/db/flist.c b/db/flist.c index c81d229ab99c..0a6cc5fcee43 100644 --- a/db/flist.c +++ b/db/flist.c @@ -424,8 +424,10 @@ flist_find_ftyp( if (f->ftyp == type) return fl; count = fcount(f, obj, startoff); - if (!count) + if (!count) { + flist_free(fl); continue; + } fa = &ftattrtab[f->ftyp]; if (fa->subfld) { flist_t *nfl;
When count is zero fl reference is lost. Fix it by freeing the list. Fixes: a0d79cb37a36 ("xfs_db: make flist_find_ftyp() to check for field existance on disk") Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> --- db/flist.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)