Re: lib/fts.c: return when malloc failed

[Pádraig Brady]

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