I found this by accident and then reproduce it through artificial mem pressure
test.
And I update the patch as you said.
*lib/fts.c：return when malloc failed in function setup_dir()
---
lib/fts.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/fts.c b/lib/fts.c
index 78584b2902..c2fa5ee8dc 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -979,7 +979,11 @@ next: tmp = p;
}
free_dir(sp);
fts_load(sp, p);
- setup_dir(sp);
+ if (! setup_dir(sp)) {
+ free_dir(sp);
+ __set_errno (ENOMEM);
+ return (NULL);
+ }
goto check_for_dir;
}
--
2.36.1
Best Regards
ChuanGang Jiang
-----邮件原件-----
发件人: Pádraig Brady <pixelb...@gmail.com> 代表 Pádraig Brady
发送时间: 2023年2月26日 23:30
收件人: ChuanGang Jiang <jiangchuangan...@outlook.com>; bug-gnulib@gnu.org
主题: Re: lib/fts.c: return when malloc failed
On 26/02/2023 14:46, ChuanGang Jiang wrote:
> Hi，‘rm’ of coreutils was terminated with signal SIGSEGV, Segmentation fault.
> Here is the backtrace with gdb.
>
> Core was generated by `/usr/bin/rm -rf test1/test2/'.
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 cycle_check (state=0x0, sb=sb@entry=0x56296ac29e00) at
> ../lib/cycle-check.c:60
> 60 assure (state->magic == CC_MAGIC);
> (gdb) bt full
> #0 cycle_check (state=0x0, sb=sb@entry=0x56296ac29e00) at
> ../lib/cycle-check.c:60
> __PRETTY_FUNCTION__ = "cycle_check"
> #1 0x000056296aa3d8bd in enter_dir (fts=0x56296ac28ae0, ent=0x56296ac29d90)
> at ../lib/fts-cycle.c:108
> st = <optimized out>
> ad = <optimized out>
> ad_from_table = <optimized out>
> #2 0x000056296aa3f031 in rpl_fts_read (sp=sp@entry=0x56296ac28ae0) at
> ../lib/fts.c:1024
> p = 0x56296ac29d90
> tmp = <optimized out>
> instr = <optimized out>
> t = <optimized out>
> #3 0x000056296aa39858 in rm (file=<optimized out>, x=0x7ffc10c71c60) at
> ../src/remove.c:597
> ent = <optimized out>
> s = <optimized out>
> bit_flags = <optimized out>
> fts = 0x56296ac28ae0
> rm_status = RM_OK
> __PRETTY_FUNCTION__ = "rm"
> #4 0x000056296aa388e2 in main (argc=<optimized out>, argv=0x7ffc10c71e88) at
> ../src/rm.c:370
> preserve_root = true
> x = {ignore_missing_files = true, interactive = RMI_NEVER,
> one_file_system = false, recursive = true, remove_empty_directories = false,
> root_dev_ino = 0x56296aa480b0 <dev_ino_buf>, preserve_all_root = false,
> stdin_tty = true, verbose = false, require_restore_cwd = false}
> prompt_once = <optimized out>
> c = <optimized out>
> n_files = <optimized out>
> file = 0x7ffc10c71e98
> status = <optimized out>
> __PRETTY_FUNCTION__ = "main"
>
>
> I think it should return when malloc failed for ’fts->fts_cycle.state‘
> in ’setup_dir(sp)‘ and the patch below may fix this.
>
>
> *lib/fts.c：return when malloc failed in function setup_dir()
> ---
> lib/fts.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/fts.c b/lib/fts.c
> index 78584b2902..374efa1be7 100644
> --- a/lib/fts.c
> +++ b/lib/fts.c
> @@ -979,7 +979,10 @@ next: tmp = p;
> }
> free_dir(sp);
> fts_load(sp, p);
> - setup_dir(sp);
> + if (! setup_dir(sp)) {
> + free_dir(sp);
> + return (NULL);
> + }
> goto check_for_dir;
> }
>
I agree that assertion may trigger if setup_dir() fails.
free_dir() is safe to call upon setup_dir() failure, so the patch looks correct.
The only way that setup_dir() can really fail is due to no memory, so I'd also
__set_errno (ENOMEM); in this case.
How did you notice this?
Did you apply artificial mem pressure for testing?
thanks!
Pádraig
