回复: lib/fts.c: return when malloc failed

[ChuanGang Jiang]

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  代表 Pádraig Brady
发送时间: 2023年2月26日 23:30
收件人: ChuanGang Jiang ; 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
> 60assure (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  0x56296aa3d8bd in enter_dir (fts=0x56296ac28ae0, ent=0x56296ac29d90) 
> at ../lib/fts-cycle.c:108
>  st = 
>  ad = 
>  ad_from_table = 
> #2  0x56296aa3f031 in rpl_fts_read (sp=sp@entry=0x56296ac28ae0) at 
> ../lib/fts.c:1024
>  p = 0x56296ac29d90
>  tmp = 
>  instr = 
>  t = 
> #3  0x56296aa39858 in rm (file=, x=0x7ffc10c71c60) at 
> ../src/remove.c:597
>  ent = 
>  s = 
>  bit_flags = 
>  fts = 0x56296ac28ae0
>  rm_status = RM_OK
>  __PRETTY_FUNCTION__ = "rm"
> #4  0x56296aa388e2 in main (argc=, 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 , preserve_all_root = false,
>stdin_tty = true, verbose = false, require_restore_cwd = false}
>  prompt_once = 
>  c = 
>  n_files = 
>  file = 0x7ffc10c71e98
>  status = 
>  __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

lib/fts.c: return when malloc failed

[ChuanGang Jiang]

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
60assure (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  0x56296aa3d8bd in enter_dir (fts=0x56296ac28ae0, ent=0x56296ac29d90) at 
../lib/fts-cycle.c:108
st = 
ad = 
ad_from_table = 
#2  0x56296aa3f031 in rpl_fts_read (sp=sp@entry=0x56296ac28ae0) at 
../lib/fts.c:1024
p = 0x56296ac29d90
tmp = 
instr = 
t = 
#3  0x56296aa39858 in rm (file=, x=0x7ffc10c71c60) at 
../src/remove.c:597
ent = 
s = 
bit_flags = 
fts = 0x56296ac28ae0
rm_status = RM_OK
__PRETTY_FUNCTION__ = "rm"
#4  0x56296aa388e2 in main (argc=, 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 , preserve_all_root = false,
  stdin_tty = true, verbose = false, require_restore_cwd = false}
prompt_once = 
c = 
n_files = 
file = 0x7ffc10c71e98
status = 
__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

[PATCH]maint: fix misspellings in comments

[ChuanGang Jiang]

*lib/tparm.c: Fix typos in comments

---
ChangeLog   |  2 +-
 lib/tparm.c | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c1ca610548..2a17679cf2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -101710,7 +101710,7 @@
(mbstok_r): Assume mbrtowc function.
* lib/propername.c: Include mbuiter.h unconditionally.
(mbsstr_trimmed_wordbounded): Assume mbrtowc function.
-   * lib/trim.c: Include mbchar.h, mbiter.h uncondtionally.
+   * lib/trim.c: Include mbchar.h, mbiter.h unconditionally.
(trim2): Assume mbrtowc function.
* lib/mbswidth.c (mbsinit): Remove fallback definition.
(mbsnwidth): Assume mbrtowc function.
diff --git a/lib/tparm.c b/lib/tparm.c
index 9913a640c1..d831bc5143 100644
--- a/lib/tparm.c
+++ b/lib/tparm.c
@@ -177,15 +177,15 @@ cvtchar (const char *sp, char *c)
Trying to handle everything has its cost, I guess.

It actually isn't to hard to figure out if a given % code is supposed
-   to be interpeted with its termcap or terminfo meaning since almost
+   to be interpreted with its termcap or terminfo meaning since almost
all terminfo codes are invalid unless something has been pushed on
the stack and termcap strings will never push things on the stack
(%p isn't used by termcap). So where we have a choice we make the
-   decision by wether or not somthing has been pushed on the stack.
+   decision by whether or not something has been pushed on the stack.
The static variable termcap keeps track of this; it starts out set
to 1 and is incremented as each argument processed by a termcap % code,
however if something is pushed on the stack it's set to 0 and the
-   rest of the % codes are interpeted as terminfo % codes. Another way
+   rest of the % codes are interpreted as terminfo % codes. Another way
of putting it is that if termcap equals one we haven't decided either
way yet, if it equals zero we're looking for terminfo codes, and if
its greater than 1 we're looking for termcap codes.
@@ -198,7 +198,7 @@ cvtchar (const char *sp, char *c)
 %c  output pop as a char
 %'c'push character constant c.
 %{n}push decimal constant n.
-%p[1-9] push paramter [1-9]
+%p[1-9] push parameter [1-9]
 %g[a-z] push variable [a-z]
 %P[a-z] put pop in variable [a-z]
 %l  push the length of pop (a string)
@@ -216,7 +216,7 @@ cvtchar (const char *sp, char *c)
 %O  logical or pop and pop and push the result
 %!  push the logical not of pop
 %? condition %t if_true [%e if_false] %;
-if condtion evaulates as true then evaluate if_true,
+if condition evaluates as true then evaluate if_true,
 else evaluate if_false. elseif's can be done:
 %? cond %t true [%e cond2 %t true2] ... [%e condN %t trueN] [%e false] 
%;
 %i  add one to parameters 1 and 2. (ANSI)
@@ -236,7 +236,7 @@ cvtchar (const char *sp, char *c)
 (UW)%sx subtract parameter FROM the character x
 %>xyif parameter > character x then add character y to parameter
 %B  convert to BCD (parameter = (parameter/10)*16 + parameter%16)
-%D  Delta Data encode (parameter = parameter - 2*(paramter%16))
+%D  Delta Data encode (parameter = parameter - 2*(parameter%16))
 %i  increment the first two parameters by one
 %n  xor the first two parameters by 0140
 (GNU)   %m  xor the first two parameters by 0177
--
2.36.1

[PATCH] hamt: Use __GNUC_MINOR__, not __GNUC_MINOR

[ChuanGang Jiang]

* lib/hamt.h:Use __GNUC_MINOR__, not  __GNUC_MINOR.
---
 lib/hamt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/hamt.h b/lib/hamt.h
index 6263b405bf..36142418d3 100644
--- a/lib/hamt.h
+++ b/lib/hamt.h
@@ -60,7 +60,7 @@ _GL_INLINE_HEADER_BEGIN
We can define it only when the compiler supports _Atomic.  For GCC,
it is supported starting with GCC 4.9.  */

-#if (__GNUC__ + (__GNUC_MINOR >= 9) > 4) \
+#if (__GNUC__ + (__GNUC_MINOR__ >= 9) > 4) \
 && __STDC_VERSION__ >= 201112L && !defined __STD_NO_ATOMICS__ \
 && !defined __cplusplus
 # define GL_HAMT_THREAD_SAFE 1
--
2.36.1

[PATCH] fix typos like "the the" and "a a" in comment

[ChuanGang Jiang]

*lib/c32is-impl.h: s/the the/the/
*lib/getopt-pfx-core.h: s/a a/a/
*lib/term-style-control.h: s/the the/the/
*lib/textstyle.in.h: s/the the/the/

---
 lib/c32is-impl.h | 2 +-
 lib/getopt-pfx-core.h| 2 +-
 lib/term-style-control.h | 2 +-
 lib/textstyle.in.h   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/c32is-impl.h b/lib/c32is-impl.h
index 5917348dc1..dba00b51ae 100644
--- a/lib/c32is-impl.h
+++ b/lib/c32is-impl.h
@@ -67,7 +67,7 @@ FUNC (wint_t wc)
   return WCHAR_FUNC (wc);
 # else
   /* The char32_t encoding of a multibyte character is known to be UCS-4,
- different from the the wchar_t encoding.  */
+ different from the wchar_t encoding.  */
   if (wc != WEOF)
 return UCS_FUNC (wc);
   else
diff --git a/lib/getopt-pfx-core.h b/lib/getopt-pfx-core.h
index 3a2fde5ad4..095e3930fe 100644
--- a/lib/getopt-pfx-core.h
+++ b/lib/getopt-pfx-core.h
@@ -47,7 +47,7 @@
 # define optind __GETOPT_ID (optind)
 # define optopt __GETOPT_ID (optopt)

-/* Work around a a problem on macOS, which declares getopt with a
+/* Work around a problem on macOS, which declares getopt with a
trailing __DARWIN_ALIAS(getopt) that would expand to something like
__asm("_" "rpl_getopt" "$UNIX2003") were it not for the following
hack to suppress the macOS declaration .  */
diff --git a/lib/term-style-control.h b/lib/term-style-control.h
index 3f853bb941..4e14c7c5b1 100644
--- a/lib/term-style-control.h
+++ b/lib/term-style-control.h
@@ -37,7 +37,7 @@ typedef enum
be left in the default state when the program is
interrupted.  */
   TTYCTL_FULL   /* Signal handling and disabling echo and 
flush-upon-signal.
-   Result: No garbled output, and the the terminal will
+   Result: No garbled output, and the terminal will
be left in the default state when the program is
interrupted.  */
 } ttyctl_t;
diff --git a/lib/textstyle.in.h b/lib/textstyle.in.h
index 111e1e10d3..38645bf6f8 100644
--- a/lib/textstyle.in.h
+++ b/lib/textstyle.in.h
@@ -392,7 +392,7 @@ typedef enum
be left in the default state when the program is
interrupted.  */
   TTYCTL_FULL   /* Signal handling and disabling echo and 
flush-upon-signal.
-   Result: No garbled output, and the the terminal will
+   Result: No garbled output, and the terminal will
be left in the default state when the program is
interrupted.  */
 } ttyctl_t;
--
2.36.1