Re: Bug#463043: address@hidden: coreutils problem]

[Jim Meyering]

Michael Stone <address@hidden> wrote:
> I can readily duplicate this bug report on a 2.6.24-rc7 kernel by
> running `ls -l /proc/sys/fs/inotify`
>
> Kernel bug? libselinux bug? Documentation bug?
>
> I suppose coreutils should check that the returned context is non-NULL.
> It looks like the debian 5.97-5.3 selinux patch (derived from redhat)
> never even looks at the return value of getfilecontext, which doesn't
> seem right, either. Does the attached patch make sense?

[ Hi Michael, thanks for forwarding that. ]

Hi Jan,

Thank you for the analysis and patch.
I preferred to do it slightly differently,
mainly to keep the work-around code in one place.

I haven't been following libselinux development, but at first glance,
libselinux1-2.0.15 seems like it must be pretty old compared to the 2.0.49
in rawhide.  And as you might expect, the unpatched ls works fine there.


        ls: don't segfault on files in /proc with an old libselinux

        * src/ls.c (gobble_file): Work around a bug in libselinux1-2.0.15
        whereby getfilecon returns 0 yet sets the context to NULL.
        Reported by Jan Moringen via Michael Stone in
        http://bugs.debian.org/463043
        * tests/ls/Makefile.am (TESTS): Add proc-selinux-segfault.
        * tests/ls/proc-selinux-segfault: Test for the above fix.
        * NEWS: Mention the fix.

diff --git a/NEWS b/NEWS
index 4811296..f474141 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 ** Bug fixes

+  ls no longer segfaults on files in /proc when linked with an older version
+  of libselinux.  E.g., ls -l /proc/sys would dereference a NULL pointer.
+
   Fix a non-portable use of sed in configure.ac.
   [bug introduced in coreutils-6.9.92]

diff --git a/THANKS b/THANKS
index f9a4f62..1e04f9b 100644
--- a/THANKS
+++ b/THANKS
@@ -225,6 +225,7 @@ James Youngman                      address@hidden
 Jamie Lokier                        address@hidden
 Jamie McClelland                    address@hidden
 Jan Fedak                           address@hidden
+Jan Moringen                        address@hidden
 Jan Nieuwenhuizen                   address@hidden
 Janos Farkas                        address@hidden
 Jarkko Hietaniemi                   address@hidden
diff --git a/src/ls.c b/src/ls.c
index 83fac90..46713f2 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1,5 +1,5 @@
 /* `dir', `vdir' and `ls' directory listing programs for GNU.
-   Copyright (C) 85, 88, 90, 91, 1995-2007 Free Software Foundation, Inc.
+   Copyright (C) 85, 88, 90, 91, 1995-2008 Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -2667,6 +2667,17 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,
                          : lgetfilecon (absolute_name, &f->scontext));
          err = (attr_len < 0);

+         /* Contrary to its documented API, getfilecon may return 0,
+            yet set f->scontext to NULL (on at least Debian's libselinux1
+            2.0.15-2+b1), so work around that bug.
+            FIXME: remove this work-around in 2011, or whenever affected
+            versions of libselinux are long gone.  */
+         if (attr_len == 0)
+           {
+             err = 0;
+             f->scontext = xstrdup ("unlabeled");
+           }
+
          if (err == 0)
            have_acl = ! STREQ ("unlabeled", f->scontext);
          else
diff --git a/tests/ls/Makefile.am b/tests/ls/Makefile.am
index 2aea419..c9739c9 100644
--- a/tests/ls/Makefile.am
+++ b/tests/ls/Makefile.am
@@ -1,6 +1,6 @@
 # Make coreutils tests for "ls".               -*-Makefile-*-

-# Copyright (C) 1997-2003, 2006-2007 Free Software Foundation, Inc.
+# Copyright (C) 1997-2003, 2006-2008 Free Software Foundation, Inc.

 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.

 TESTS = \
+  proc-selinux-segfault \
   stat-free-symlinks \
   nameless-uid \
   color-dtype-dir \
diff --git a/tests/ls/proc-selinux-segfault b/tests/ls/proc-selinux-segfault
new file mode 100755
index 0000000..320ba6f
--- /dev/null
+++ b/tests/ls/proc-selinux-segfault
@@ -0,0 +1,33 @@
+#!/bin/sh
+# ls -l /proc/sys would segfault when built against libselinux1 2.0.15-2+b1
+
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  ls --version
+fi
+
+. $srcdir/../envvar-check
+. $srcdir/../test-lib.sh
+
+f=/proc/sys
+test -r $f || f=.
+
+fail=0
+ls -l $f > out || fail=1
+
+(exit $fail); exit $fail
--
1.5.4.rc5.1.ge6bfe