* [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