Re: [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init()

Hi Utkal,

Thanks for the review. You're right on both points — v2 will use
erofs_io_close() and fix the indentation.

Thanks, Deepak Pathik

On Thu, Apr 2, 2026 at 9:58 AM Utkal Singh <[email protected]> wrote:

> On Thu, 02 Apr 2026 01:10, Deepak Pathik wrote:
> > +if (!m2gr->bmgr) {
> > +close(m2gr->vf.fd);
>
> erofs_io_close() does more than close(fd) — it dispatches through
> vf->ops->close(vf) if ops is set, and resets vf->fd to -1 afterward.
> Using raw close() here skips both, which is incorrect.
>
> Also, the if block is missing tab indentation.
>
> Suggested fix:
>
> if (!m2gr->bmgr) {
> erofs_io_close(&m2gr->vf);
> return -ENOMEM;
> }
>
> On Thu, 2 Apr 2026 at 01:10, Deepak Pathik <[email protected]>
> wrote:
> >
> > In erofs_metamgr_init(), erofs_tmpfile() returns a file
> > descriptor stored in m2gr->vf.fd. If the subsequent
> > erofs_buffer_init() call fails, the function returns -ENOMEM
> > without closing this file descriptor.
> >
> > The caller erofs_metadata_init() handles this failure at
> > err_free, which only frees the m2gr struct. The fd is
> > therefore leaked with no remaining reference to close it.
> >
> > The success path correctly cleans up via erofs_metamgr_exit(),
> > which calls erofs_io_close(&m2gr->vf). Mirror that behaviour
> > on the error path by closing the fd before returning.
> >
> > Signed-off-by: Deepak Pathik <[email protected]>
> > ---
> >  lib/metabox.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/metabox.c b/lib/metabox.c
> > index 12706aa..d55e787 100644
> > --- a/lib/metabox.c
> > +++ b/lib/metabox.c
> > @@ -32,8 +32,10 @@ static int erofs_metamgr_init(struct erofs_sb_info
> *sbi,
> >
> >  m2gr->vf = (struct erofs_vfile){ .fd = ret };
> >         m2gr->bmgr = erofs_buffer_init(sbi, 0, &m2gr->vf);
> > - if (!m2gr->bmgr)
> > +if (!m2gr->bmgr) {
> > +close(m2gr->vf.fd);
> >                 return -ENOMEM;
> > +}
> >         return 0;
> >  }
> > --
> > 2.50.1
> >
> >
> >
>