Re: new snapshot available, ls -Z crash

[Pádraig Brady]

On 17/01/2025 16:17, Pádraig Brady wrote:
> On 17/01/2025 16:00, Bruno Haible wrote:
> > On
> >     - OpenBSD 6.0,
> >     - NetBSD 7.1
> > I see a test failure:
> >
> >     FAIL tests/ls/selinux-segfault.sh (exit status: 1)

Note I couldn't repro the failure on cfarm220 at least.
But yes I do see problems in this code ...

> > Log is attached.
> >
> > The cause is a crash that is easy to reproduce interactively:
> >
> >     $ src/ls -Z .
> >
> > In gdb, I get this stack trace (same on both platforms):
> >     #0 strlen
> >     #1 gobble_file (name = ".", type=arg_directory, inode=0, 
> > command_line_arg=true, dirname=0x0)
> >        at ls.c:3627
> >     #2 main () at ls.c:1812
> >
> > The problem is that file_has_aclinfo_cache (called from ls.c:3526)
> > has returned
> >     ai = { size = 0, scontext = 0x0, scontext_err = 0, ... }
>
> Hmm. So these platforms must not set an error,
> but also return no context.  I suppose something like
> this untested gnulib patch would help:
>
> diff --git a/lib/file-has-acl.c b/lib/file-has-acl.cindex 
> c02cfee842..cda7012408 100644
> --- a/lib/file-has-acl.c
> +++ b/lib/file-has-acl.c
> @@ -253,7 +253,7 @@ get_aclinfo (char const *name, struct aclinfo *ai, int 
> flags)
>            }
>        }
>      ai->scontext_err = scontext_err;
> -  if (scontext_err)
> +  if (scontext_err || !ai->scontext)
>        ai->scontext = (char *) UNKNOWN_SECURITY_CONTEXT;
>    }
>
> Given the issue seems restricted to these platforms,
> the impact on just released coreutils 9.6 should be minimal.

Le sigh. file_has_aclinfo_cache() has more issues.
`ls -Z .` seems to segfault on any system!

`ls -lZ foo` or `ls -Z file` are ok they do the stat for each file,
but `ls -Z .` does not, so it always treats files as unsupported,
and then crashes accessing NULL contexts.

This avoids that crash at least.

diff --git a/src/ls.c b/src/ls.c
index 321536021..366eda084 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1768,7 +1768,7 @@ main (int argc, char **argv)

   format_needs_stat = ((sort_type == sort_time) | (sort_type == sort_size)
                        | (format == long_format)
-                       | print_block_size | print_hyperlink);
+                       | print_block_size | print_hyperlink | print_scontext);
   format_needs_type = ((! format_needs_stat)
                        & (recursive | print_with_color | print_scontext
                           | directories_first
@@ -3309,7 +3309,7 @@ file_has_aclinfo_cache (char const *file, struct fileinfo 
*f,
   static int unsupported_scontext_err;
   static dev_t unsupported_device;

-  if (f->stat.st_dev == unsupported_device)
+  if (f->stat_ok && f->stat.st_dev == unsupported_device)
     {
       ai->buf = ai->u.__gl_acl_ch;
       ai->size = 0;


But we'll need to handle the failure mode you mentioned,
and We'll need to add a test also.

Pádraig