public inbox for linux-erofs@ozlabs.org
 help / color / mirror / Atom feed
* [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init()
@ 2026-04-01 19:40 Deepak Pathik
  2026-04-02  4:28 ` Utkal Singh
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Deepak Pathik @ 2026-04-01 19:40 UTC (permalink / raw)
  To: linux-erofs; +Cc: hsiangkao, xiang, deepakpathik2005

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 <deepakpathik2005@gmail.com>
---
 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




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init()
  2026-04-01 19:40 [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init() Deepak Pathik
@ 2026-04-02  4:28 ` Utkal Singh
  2026-04-02  6:23   ` Deepak Pathik
  2026-04-02 11:20 ` [PATCH v2] " Deepak Pathik
  2026-04-02 11:26 ` [PATCH v3] " Deepak Pathik
  2 siblings, 1 reply; 5+ messages in thread
From: Utkal Singh @ 2026-04-02  4:28 UTC (permalink / raw)
  To: Deepak Pathik; +Cc: linux-erofs, hsiangkao, xiang

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 <deepakpathik2005@gmail.com> 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 <deepakpathik2005@gmail.com>
> ---
>  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
>
>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init()
  2026-04-02  4:28 ` Utkal Singh
@ 2026-04-02  6:23   ` Deepak Pathik
  0 siblings, 0 replies; 5+ messages in thread
From: Deepak Pathik @ 2026-04-02  6:23 UTC (permalink / raw)
  To: Utkal Singh; +Cc: linux-erofs, hsiangkao, xiang

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]

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 <singhutkal015@gmail.com> 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 <deepakpathik2005@gmail.com>
> 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 <deepakpathik2005@gmail.com>
> > ---
> >  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
> >
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 3175 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] erofs-utils: lib: fix fd leak in erofs_metamgr_init()
  2026-04-01 19:40 [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init() Deepak Pathik
  2026-04-02  4:28 ` Utkal Singh
@ 2026-04-02 11:20 ` Deepak Pathik
  2026-04-02 11:26 ` [PATCH v3] " Deepak Pathik
  2 siblings, 0 replies; 5+ messages in thread
From: Deepak Pathik @ 2026-04-02 11:20 UTC (permalink / raw)
  To: linux-erofs; +Cc: hsiangkao, xiang, Deepak Pathik

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 <deepakpathik2005@gmail.com>
---
v2: use erofs_io_close() instead of raw close(); rebased on latest upstream/dev

 lib/metabox.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/metabox.c b/lib/metabox.c
index d5ce9e3..86a7083 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) {
+erofs_io_close(&m2gr->vf);
 		return -ENOMEM;
+}
 	return 0;
 }
 
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3] erofs-utils: lib: fix fd leak in erofs_metamgr_init()
  2026-04-01 19:40 [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init() Deepak Pathik
  2026-04-02  4:28 ` Utkal Singh
  2026-04-02 11:20 ` [PATCH v2] " Deepak Pathik
@ 2026-04-02 11:26 ` Deepak Pathik
  2 siblings, 0 replies; 5+ messages in thread
From: Deepak Pathik @ 2026-04-02 11:26 UTC (permalink / raw)
  To: linux-erofs; +Cc: hsiangkao, xiang

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 <deepakpathik2005@gmail.com>
---
 lib/metabox.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/metabox.c b/lib/metabox.c
index d5ce9e3..86a7083 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) {
+		erofs_io_close(&m2gr->vf);
 		return -ENOMEM;
+	}
 	return 0;
 }
 
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-02 11:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-01 19:40 [PATCH] erofs-utils: lib: fix fd leak in erofs_metamgr_init() Deepak Pathik
2026-04-02  4:28 ` Utkal Singh
2026-04-02  6:23   ` Deepak Pathik
2026-04-02 11:20 ` [PATCH v2] " Deepak Pathik
2026-04-02 11:26 ` [PATCH v3] " Deepak Pathik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox