bug#21130: test fail: 'tests/ls/stat-failed'

[Pádraig Brady]

On 24/07/15 22:46, Assaf Gordon wrote:
> Hello,
> 
> checking coreutils 8.24, running on NFS, the test 'ls/stat-failed' fails (log 
> attached).
> 
> If I understand correctly,
> The test creates a symlink to a directory then removes execute permissions:
> mkdir d
> ln -s / d/s
> chmod 600 d
> 
> Then tries to dereference it:
>  $ ls -Log d
>  ls: cannot access d/s: Permission denied
>  total 0
>  d? ? ?? s
> 
> The test expect 's' to have 'l' type, on my system it is 'd'.
> 
> The attached patch avoids the failure, though I don't know if this is correct 
> or not (perhaps this failure should not be avoided?).
> 
> The system is:
>  $ uname -a
>  Linux XXX 2.6.32-431.el6.x86_64 #1 SMP Fri Nov 22 03:15:09 UTC 2013 
> x86_64 x86_64 x86_64 GNU/Linux
>  $ gcc --version
>   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-4)
> 
> The file system is NFS.

False failure documented and avoided in the attached.

thanks,
Pádraig.

>From 56d3269207db13b0f3a594d6d2cbc52913929d21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sat, 21 Nov 2015 10:59:37 +
Subject: [PATCH] tests: avoid false failure on older NFS implementations

* tests/ls/stat-failed.sh: Skip the test if 'd' is returned as the type,
and document where this was seen.  Also flag failure to write small
temp files during the test as an error rather than a failure.
Fixes http://bugs.gnu.org/21130
---
 tests/ls/stat-failed.sh | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/ls/stat-failed.sh b/tests/ls/stat-failed.sh
index 002b71c..32e8304 100755
--- a/tests/ls/stat-failed.sh
+++ b/tests/ls/stat-failed.sh
@@ -31,7 +31,11 @@ chmod 600 d || framework_failure_
 ls -Log d > out
 test $? = 1 || fail=1
 
-cat <<\EOF > exp || fail=1
+# Linux 2.6.32 client with Isilon OneFS always returns d_type==DT_DIR ('d')
+# Newer Linux 3.10.0 returns the more correct DT_UNKNOWN ('?')
+grep '^[l?]?' out || skip_ 'unrecognized d_type returned'
+
+cat <<\EOF > exp || framework_failure_
 total 0
 ?? ? ?? s
 EOF
@@ -42,7 +46,7 @@ sed 's/^l/?/' out | compare exp - || fail=1
 rm -f out exp
 returns_ $LS_MINOR_PROBLEM ls --dired -l d > out || fail=1
 
-cat <<\EOF > exp || fail=1
+cat <<\EOF > exp || framework_failure_
   total 0
   ?? ? ? ? ?? s
 //DIRED// 44 45
-- 
2.5.0

bug#21130: test fail: 'tests/ls/stat-failed'

[Paul Eggert]

Pádraig Brady wrote:

On 24/07/15 22:46, Assaf Gordon wrote:

If I understand correctly,
The test creates a symlink to a directory then removes execute permissions:
 mkdir d
 ln -s / d/s
 chmod 600 d

Then tries to dereference it:
  $ ls -Log d
  ls: cannot access d/s: Permission denied
  total 0
  d? ? ?? s



Another possibility might be that the chmod(1) and stat(2) are racy
thus allowing the stat() to succeed? If that was the case then
a stat d/s  skip_ ... would avoid the false failure?


Sorry, I don't understand the scenario here.  If the stat succeeds, why would ls 
output ''?  The '' means that the stat failed.

bug#21130: test fail: 'tests/ls/stat-failed'

[Pádraig Brady]

On 25/07/15 16:05, Paul Eggert wrote:
 Pádraig Brady wrote:
 On 24/07/15 22:46, Assaf Gordon wrote:
 If I understand correctly,
 The test creates a symlink to a directory then removes execute permissions:
  mkdir d
  ln -s / d/s
  chmod 600 d

 Then tries to dereference it:
   $ ls -Log d
   ls: cannot access d/s: Permission denied
   total 0
   d? ? ?? s
 
 Another possibility might be that the chmod(1) and stat(2) are racy
 thus allowing the stat() to succeed? If that was the case then
 a stat d/s  skip_ ... would avoid the false failure?
 
 Sorry, I don't understand the scenario here.  If the stat succeeds, why would 
 ls 
 output ''?  The '' means that the stat failed.

Yes good point. So it must be that d_type is set to 'd' erroneously.
What is the file system and operating system on the NFS server?

bug#21130: test fail: 'tests/ls/stat-failed'

[Assaf Gordon]

Hello,

 On Jul 25, 2015, at 13:13, Pádraig Brady p...@draigbrady.com wrote:

 On 24/07/15 22:46, Assaf Gordon wrote:
 Then tries to dereference it:
  $ ls -Log d
  ls: cannot access d/s: Permission denied
  total 0
  d? ? ?? s
 
...

 Yes good point. So it must be that d_type is set to 'd' erroneously.
 What is the file system and operating system on the NFS server?

The NFS server is an Isilon OneFS (not sure which model/version).

strace gives:
  stat(d, {st_mode=S_IFDIR|0600, st_size=19, ...}) = 0
  open(d, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
  fcntl(3, F_GETFD) = 0x1 (flags FD_CLOEXEC)
  getdents(3, /* 3 entries */, 32768) = 72
  stat(d/s, 0x1fc9690) = -1 EACCES (Permission denied) 

Breaking in 'print_dir()' (ls.c:2612) gives:

---
##
## The code was next = readdir(dirp)
##
(gdb) p *next
$9 = {d_ino = 6682814318, d_off = 3, d_reclen = 24, d_type = 4 '\004',
  d_name = s\000\000\000\004, '\000' repeats 250 times}

So it seems the filesystem reports d_type==DT_DIR.


For completeness, I've tested with other file types, and it seems that the 
returned type is always 'directory':
---
$ chmod 700 d
$ ls -log d
total 8
lrwxrwxrwx 1  9 Jul 25 18:29 blockdev - /dev/sda1
lrwxrwxrwx 1  9 Jul 25 18:29 chardev - /dev/tty0
lrwxrwxrwx 1 11 Jul 25 18:29 file - /etc/passwd
lrwxrwxrwx 1  1 Jul 24 17:41 s - /

$ ls -Log d
total 8
brw-rw  1 8, 1 Dec 16  2014 blockdev
crw--w  1 4, 0 Dec 16  2014 chardev
-rw-r--r--  1 1808 Jun 26 12:00 file
dr-xr-xr-x 26 4096 Jan 29 15:06 s

$ chmod 600 d
$ ls -Log d
ls: cannot access d/file: Permission denied
ls: cannot access d/chardev: Permission denied
ls: cannot access d/blockdev: Permission denied
ls: cannot access d/s: Permission denied
total 0
d? ? ?? blockdev
d? ? ?? chardev
d? ? ?? file
d? ? ?? s
---

regards,
 - assaf

bug#21130: test fail: 'tests/ls/stat-failed'

[Assaf Gordon]

On 07/25/2015 07:57 PM, Paul Eggert wrote:

Assaf Gordon wrote:

it seems that the returned type is always 'directory':

...

Is the bug reproducible on a more-modern Linux kernel?

...

If the bug doesn't appear in 3.2 or newer, I wouldn't worry about it.


Thanks for the pointer - on another machine with newer kernel (mounting the 
same NFS share) the reported type is 'unknown'.

===
$  uname -a
Linux  3.10.0-229.el7.x86_64 #1 SMP Fri Mar 6 11:36:42 UTC 2015 x86_64 
GNU/Linux

$ ./src/ls -Log d
./src/ls: cannot access d/file: Permission denied
./src/ls: cannot access d/chardev: Permission denied
./src/ls: cannot access d/blockdev: Permission denied
./src/ls: cannot access d/s: Permission denied
total 0
?? ? ?? blockdev
?? ? ?? chardev
?? ? ?? file
?? ? ?? s
===

bug#21130: test fail: 'tests/ls/stat-failed'

[Paul Eggert]

Assaf Gordon wrote:

it seems that the returned type is always 'directory':


Wow,that's quite a bug.  I imagine it affects programs other than 'ls'.  Sounds 
like it needs to be fixed in the file system or kernel, as it's not realistic to 
install workarounds in every application that uses readdir.


Is the bug reproducible on a more-modern Linux kernel?  2.6.32 is projected for 
end-of-life around now, according to 
https://www.kernel.org/category/releases.html.  If the bug doesn't appear in 
3.2 or newer, I wouldn't worry about it.


I found a machine with an old kernel (RHEL 6.6 with kernel 
2.6.32-504.12.2.el6.x86_64) and could not reproduce the bug with a Network 
Appliance NFS server.  So perhaps the bug is Isilon-specific rather than 
kernel-specific.  If so, Isilon should fix it.

bug#21130: test fail: 'tests/ls/stat-failed'

[Paul Eggert]

Assaf Gordon wrote:

$ ./src/ls -Log d
./src/ls: cannot access d/file: Permission denied
./src/ls: cannot access d/chardev: Permission denied
./src/ls: cannot access d/blockdev: Permission denied
./src/ls: cannot access d/s: Permission denied
total 0
?? ? ?? blockdev


Arguably this is a better result than what we normally get, and our test 
framework should allow it.


The argument is that if a directory is not searchable, an application should be 
able to discover its files' names (as they belong to the directory and not to 
the files), but not the files' attributes (as they belong to the files, not to 
the directory).  Admittedly this is a fine distinction.

bug#21130: test fail: 'tests/ls/stat-failed'

[Assaf Gordon]

Hello,

checking coreutils 8.24, running on NFS, the test 'ls/stat-failed' fails (log 
attached).

If I understand correctly,
The test creates a symlink to a directory then removes execute permissions:
   mkdir d
   ln -s / d/s
   chmod 600 d

Then tries to dereference it:
$ ls -Log d
ls: cannot access d/s: Permission denied
total 0
d? ? ?? s

The test expect 's' to have 'l' type, on my system it is 'd'.

The attached patch avoids the failure, though I don't know if this is correct 
or not (perhaps this failure should not be avoided?).

The system is:
$ uname -a
Linux XXX 2.6.32-431.el6.x86_64 #1 SMP Fri Nov 22 03:15:09 UTC 2013 x86_64 
x86_64 x86_64 GNU/Linux
$ gcc --version
 gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-4)

The file system is NFS.


regards,
 - assaf

   GNU coreutils 8.24: ./tests/test-suite.log


# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: tests/ls/stat-failed
==

++ initial_cwd_=/data/research/erlich_lab/gordon/sources/coreutils-8.24
++ fail=0
+++ testdir_prefix_
+++ printf gt
++ pfx_=gt
+++ mktempd_ /data/research/erlich_lab/gordon/sources/coreutils-8.24 gt-stat-failed.sh.
+++ case $# in
+++ destdir_=/data/research/erlich_lab/gordon/sources/coreutils-8.24
+++ template_=gt-stat-failed.sh.
+++ MAX_TRIES_=4
+++ case $destdir_ in
+++ case $template_ in
 unset TMPDIR
+++ d=/data/research/erlich_lab/gordon/sources/coreutils-8.24/gt-stat-failed.sh.LdZJ
+++ case $d in
+++ test -d /data/research/erlich_lab/gordon/sources/coreutils-8.24/gt-stat-failed.sh.LdZJ
 tr S -
+++ perms='drwx-- 2 0 Jul 24 16:48 /data/research/erlich_lab/gordon/sources/coreutils-8.24/gt-stat-failed.sh.LdZJ'
+++ case $perms in
+++ test 0 = 0
+++ echo /data/research/erlich_lab/gordon/sources/coreutils-8.24/gt-stat-failed.sh.LdZJ
+++ return
++ test_dir_=/data/research/erlich_lab/gordon/sources/coreutils-8.24/gt-stat-failed.sh.LdZJ
++ cd /data/research/erlich_lab/gordon/sources/coreutils-8.24/gt-stat-failed.sh.LdZJ
++ gl_init_sh_nl_='
'
++ IFS=' 	
'
++ for sig_ in 1 2 3 13 15
+++ expr 1 + 128
++ eval 'trap '\''Exit 129'\'' 1'
+++ trap 'Exit 129' 1
++ for sig_ in 1 2 3 13 15
+++ expr 2 + 128
++ eval 'trap '\''Exit 130'\'' 2'
+++ trap 'Exit 130' 2
++ for sig_ in 1 2 3 13 15
+++ expr 3 + 128
++ eval 'trap '\''Exit 131'\'' 3'
+++ trap 'Exit 131' 3
++ for sig_ in 1 2 3 13 15
+++ expr 13 + 128
++ eval 'trap '\''Exit 141'\'' 13'
+++ trap 'Exit 141' 13
++ for sig_ in 1 2 3 13 15
+++ expr 15 + 128
++ eval 'trap '\''Exit 143'\'' 15'
+++ trap 'Exit 143' 15
++ trap remove_tmp_ 0
+ path_prepend_ ./src
+ test 1 '!=' 0
+ path_dir_=./src
+ case $path_dir_ in
+ abs_path_dir_=/data/research/erlich_lab/gordon/sources/coreutils-8.24/./src
+ case $abs_path_dir_ in
+ PATH=/data/research/erlich_lab/gordon/sources/coreutils-8.24/./src:/data/research/erlich_lab/gordon/sources/coreutils-8.24/src:/nethome/agordon/usr/bin:/opt/sge/bin:/opt/sge/bin/lx-amd64:/usr/lib64/qt-3.3/bin:/usr/local/bin:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin
+ create_exe_shims_ /data/research/erlich_lab/gordon/sources/coreutils-8.24/./src
+ case $EXEEXT in
+ return 0
+ shift
+ test 0 '!=' 0
+ export PATH
+ print_ver_ ls
+ test yes = yes
+ local i
+ for i in '$*'
+ env ls --version
ls (GNU coreutils) 8.24
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Richard M. Stallman and David MacKenzie.
+ skip_if_root_
+ uid_is_privileged_
++ id -u
+ my_uid=10346
+ case $my_uid in
+ return 1
+ LS_MINOR_PROBLEM=1
+ mkdir d
+ ln -s / d/s
+ chmod 600 d
+ ls -Log d
ls: cannot access d/s: Permission denied
+ test 1 = 1
+ cat
+ sed 's/^l/?/' out
+ compare exp -
+ compare_dev_null_ exp -
+ test 2 = 2
+ test xexp = x/dev/null
+ test x- = x/dev/null
+ return 2
+ case $? in
+ compare_ exp -
+ diff -u exp -
--- exp	2015-07-24 16:48:36.152677768 -0400
+++ -	2015-07-24 16:48:36.172187132 -0400
@@ -1,2 +1,2 @@
 total 0
-?? ? ?? s
+d? ? ?? s
+ fail=1
+ rm -f out exp
+ returns_ 1 ls --dired -l d
ls: cannot access d/s: Permission denied
+ cat
+ sed 's/^  l/  ?/' out
+ compare exp -
+ compare_dev_null_ exp -
+ test 2 = 2
+ test xexp = x/dev/null
+ test x- = x/dev/null
+ return 2
+ case $? in
+ compare_ exp -
+ diff -u exp -
--- exp	2015-07-24 16:48:36.262522027 -0400
+++ -	2015-07-24 16:48:36.270381375 -0400
@@ -1,4 +1,4 @@
   total 0
-  ?? ? ? ? ?? s
+  d? ? ? ? ?? s
 //DIRED// 44 45
 //DIRED-OPTIONS// --quoting-style=literal
+ fail=1
+ Exit 1
+ set +e
+ exit 1
+ exit 1
+ remove_tmp_
+ __st=1
+ cleanup_
+ :
+ cd /data/research/erlich_lab/gordon/sources/coreutils-8.24

bug#21130: test fail: 'tests/ls/stat-failed'

[Pádraig Brady]

On 24/07/15 22:46, Assaf Gordon wrote:
 Hello,
 
 checking coreutils 8.24, running on NFS, the test 'ls/stat-failed' fails (log 
 attached).
 
 If I understand correctly,
 The test creates a symlink to a directory then removes execute permissions:
 mkdir d
 ln -s / d/s
 chmod 600 d
 
 Then tries to dereference it:
  $ ls -Log d
  ls: cannot access d/s: Permission denied
  total 0
  d? ? ?? s
 
 The test expect 's' to have 'l' type, on my system it is 'd'.
 
 The attached patch avoids the failure, though I don't know if this is correct 
 or not (perhaps this failure should not be avoided?).
 
 The system is:
  $ uname -a
  Linux XXX 2.6.32-431.el6.x86_64 #1 SMP Fri Nov 22 03:15:09 UTC 2013 
 x86_64 x86_64 x86_64 GNU/Linux
  $ gcc --version
   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-4)
 
 The file system is NFS.

Interesting. Is the NFS server RHEL6 too?

With normal perms on 'd':
 On ext4 I get ^d
  With syscalls being:
getdents(3, /* 3 entries */, 32768) = 72
stat(d/s, {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0

With -x perms on 'd':
 On nfsv4 here I get ^?
 On ext4 I get ^l
  With syscalls being:
getdents(3, /* 3 entries */, 32768) = 72
stat(d/s, 0x7f634b9594a0) = -1 EACCES (Permission denied)

So it seems you're getting next-d_type==DR_DIR in that case?
That would be wrong, but a system issue rather than a problem in ls I think.

The test adjustment looks OK but would benefit from a comment
stating the first letter represents d_type and that on buggy
systems may be 'd'.

thanks!
Pádraig

bug#21130: test fail: 'tests/ls/stat-failed'

[Pádraig Brady]

On 25/07/15 06:07, Pádraig Brady wrote:
 On 24/07/15 22:46, Assaf Gordon wrote:
 Hello,

 checking coreutils 8.24, running on NFS, the test 'ls/stat-failed' fails 
 (log attached).

 If I understand correctly,
 The test creates a symlink to a directory then removes execute permissions:
 mkdir d
 ln -s / d/s
 chmod 600 d

 Then tries to dereference it:
  $ ls -Log d
  ls: cannot access d/s: Permission denied
  total 0
  d? ? ?? s

 The test expect 's' to have 'l' type, on my system it is 'd'.

 The attached patch avoids the failure, though I don't know if this is 
 correct or not (perhaps this failure should not be avoided?).

 The system is:
  $ uname -a
  Linux XXX 2.6.32-431.el6.x86_64 #1 SMP Fri Nov 22 03:15:09 UTC 2013 
 x86_64 x86_64 x86_64 GNU/Linux
  $ gcc --version
   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-4)

 The file system is NFS.
 
 Interesting. Is the NFS server RHEL6 too?
 
 With normal perms on 'd':
  On ext4 I get ^d
   With syscalls being:
 getdents(3, /* 3 entries */, 32768) = 72
 stat(d/s, {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
 
 With -x perms on 'd':
  On nfsv4 here I get ^?
  On ext4 I get ^l
   With syscalls being:
 getdents(3, /* 3 entries */, 32768) = 72
 stat(d/s, 0x7f634b9594a0) = -1 EACCES (Permission denied)
 
 So it seems you're getting next-d_type==DR_DIR in that case?
 That would be wrong, but a system issue rather than a problem in ls I think.
 
 The test adjustment looks OK but would benefit from a comment
 stating the first letter represents d_type and that on buggy
 systems may be 'd'.

Another possibility might be that the chmod(1) and stat(2) are racy
thus allowing the stat() to succeed? If that was the case then
a stat d/s  skip_ ... would avoid the false failure?