* lib/fts.c: return when malloc failed
@ 2023-02-26 14:46 ChuanGang Jiang
2023-02-26 15:29 ` Pádraig Brady
0 siblings, 1 reply; 4+ messages in thread
From: ChuanGang Jiang @ 2023-02-26 14:46 UTC (permalink / raw)
To: bug-gnulib@gnu.org
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;
}
--
2.36.1
Best Regards
ChuanGang Jiang
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: lib/fts.c: return when malloc failed
2023-02-26 14:46 lib/fts.c: return when malloc failed ChuanGang Jiang
@ 2023-02-26 15:29 ` Pádraig Brady
2023-02-27 11:36 ` 回复: " ChuanGang Jiang
0 siblings, 1 reply; 4+ messages in thread
From: Pádraig Brady @ 2023-02-26 15:29 UTC (permalink / raw)
To: ChuanGang Jiang, bug-gnulib@gnu.org
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* 回复: lib/fts.c: return when malloc failed
2023-02-26 15:29 ` Pádraig Brady
@ 2023-02-27 11:36 ` ChuanGang Jiang
2023-02-27 12:02 ` Pádraig Brady
0 siblings, 1 reply; 4+ messages in thread
From: ChuanGang Jiang @ 2023-02-27 11:36 UTC (permalink / raw)
To: Pádraig Brady, bug-gnulib@gnu.org
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 <pixelbeat@gmail.com> 代表 Pádraig Brady
发送时间: 2023年2月26日 23:30
收件人: ChuanGang Jiang <jiangchuanganghw@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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: 回复: lib/fts.c: return when malloc failed
2023-02-27 11:36 ` 回复: " ChuanGang Jiang
@ 2023-02-27 12:02 ` Pádraig Brady
0 siblings, 0 replies; 4+ messages in thread
From: Pádraig Brady @ 2023-02-27 12:02 UTC (permalink / raw)
To: ChuanGang Jiang, bug-gnulib@gnu.org
On 27/02/2023 11:36, ChuanGang Jiang wrote:
> 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;
> }
>
Pushed.
I'll also apply this to the upcoming coreutils release for rm.
thanks,
Pádraig.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-27 12:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-26 14:46 lib/fts.c: return when malloc failed ChuanGang Jiang
2023-02-26 15:29 ` Pádraig Brady
2023-02-27 11:36 ` 回复: " ChuanGang Jiang
2023-02-27 12:02 ` Pádraig Brady
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).