On 12/22/21 09:34, Andrea Monaco wrote: > Hello, > > when searching by inode number (-inum predicate) on my GNU/Hurd system, > find crashes due to assertion "p->st_ino" in get_info failing. > > The same assertion is also done in pred.c at line 501. Inode numbers > are often assumed to start from one, but apparently this is not mandated > by POSIX and is not true on my system. > > Do that assertions protect against some type of errors? Otherwise we > could remove them. > > Thanks, > Andrea Monaco
Thanks for the bug report. I managed to reproduce in a local GNU/Hurd VM on VirtualBox, see: https://www.researchut.com/blog/debian-hurd-on-virtualbox/ Indeed, GNU/Hurd is using inode number Zero e.g. for /dev/console and /dev/tty. The first patch fixes a side effect of '-D stat' on SELinux handling which I found while reading adjacent code; pushed at: https://git.sv.gnu.org/cgit/findutils.git/commit/?id=68979e578 The 2nd patch fixes the reported issue; pushed at: https://git.sv.gnu.org/cgit/findutils.git/commit/?id=9290525c7 Have a nice day, Berny
From 68979e578cbea71211af45685561df52232623d1 Mon Sep 17 00:00:00 2001 From: Bernhard Voelker <m...@bernhard-voelker.de> Date: Sat, 25 Dec 2021 16:14:59 +0100 Subject: [PATCH 1/2] find: avoid '-D stat' side effect on SELinux handling with -L or -H With the '-D stat' debugging option turned on, 'find -L' determined the SELinux context information as if the -L (or -H) option was not given: $ strace -ve getxattr,lgetxattr find -L . -maxdepth 0 -printf '%Z:%p\n' getxattr(".", "security.selinux", 0x55b29a2b2d40, 255) = -1 ENODATA (No data available) ... $ strace -ve getxattr,lgetxattr find -D stat -L . -maxdepth 0 -printf '%Z:%p\n' lgetxattr(".", "security.selinux", 0x5649c91d8d40, 255) = -1 ENODATA (No data available) ... * find/parser.c (set_follow_state): Move the DebugStat handling after the switch statement, thus eliminating the if/else. Bug present since adding the SELinux implementation in v4.5.5-42-g1a05af6a. --- NEWS | 6 ++++++ find/parser.c | 49 ++++++++++++++++++++++++------------------------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index 02c1c55d..046eaddb 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,12 @@ GNU findutils NEWS - User visible changes. -*- outline -*- (allout) update now skips (fuse-mounted) s3fs filesystems by default, i.e., unless PRUNEFS is set. +** Bug Fixes + + 'find -D stat -L ...' no longer determines SELinux security information as + if the -L option was not given. + [Bug present since the SELinux implementation in 4.5.6] + ** Documentation Changes The find.1 man page and the Texinfo manual now show environment variables diff --git a/find/parser.c b/find/parser.c index f5d100a2..454764f3 100644 --- a/find/parser.c +++ b/find/parser.c @@ -503,6 +503,30 @@ get_stat_Ytime (const struct stat *p, void set_follow_state (enum SymlinkOption opt) { + switch (opt) + { + case SYMLINK_ALWAYS_DEREF: /* -L */ + options.xstat = optionl_stat; + options.x_getfilecon = optionl_getfilecon; + options.no_leaf_check = true; + break; + + case SYMLINK_NEVER_DEREF: /* -P (default) */ + options.xstat = optionp_stat; + options.x_getfilecon = optionp_getfilecon; + /* Can't turn no_leaf_check off because the user might have specified + * -noleaf anyway + */ + break; + + case SYMLINK_DEREF_ARGSONLY: /* -H */ + options.xstat = optionh_stat; + options.x_getfilecon = optionh_getfilecon; + options.no_leaf_check = true; + } + + options.symlink_handling = opt; + if (options.debug_options & DebugStat) { /* For DebugStat, the choice is made at runtime within debug_stat() @@ -510,31 +534,6 @@ set_follow_state (enum SymlinkOption opt) */ options.xstat = debug_stat; } - else - { - switch (opt) - { - case SYMLINK_ALWAYS_DEREF: /* -L */ - options.xstat = optionl_stat; - options.x_getfilecon = optionl_getfilecon; - options.no_leaf_check = true; - break; - - case SYMLINK_NEVER_DEREF: /* -P (default) */ - options.xstat = optionp_stat; - options.x_getfilecon = optionp_getfilecon; - /* Can't turn no_leaf_check off because the user might have specified - * -noleaf anyway - */ - break; - - case SYMLINK_DEREF_ARGSONLY: /* -H */ - options.xstat = optionh_stat; - options.x_getfilecon = optionh_getfilecon; - options.no_leaf_check = true; - } - } - options.symlink_handling = opt; } -- 2.34.1
From 9290525c774290e451b13f7cdf81262d7656e3ed Mon Sep 17 00:00:00 2001 From: Bernhard Voelker <m...@bernhard-voelker.de> Date: Sun, 26 Dec 2021 11:09:22 +0100 Subject: [PATCH 2/2] find: fix visiting of files with inode number Zero On GNU/Hurd, the value 0 is a valid inode number, and is e.g. used for /dev/console and /dev/tty. The find(1) program aborted on this platform when the user specified the -inum test and when the search visited such a file. $ find /dev/null /dev/tty -inum 40799 -printf '%i:%p\n' 40799:/dev/null find: util.c:330: get_info: Assertion `p->st_ino' failed. Aborted Likewise, 'find -printf %i' aborted when hitting such a file. * find/defs.h (get_info): Remove declaration. * find/pred.c (pred_inum): Remove the redundant assert for ST_INO as parse_inum sets need_inum=true which ensures that the inode number is known. * find/util.c (get_info): Declare static, and simplify: remove the assertions for the inode number and file type. While at it, add condition !state.have_stat in the need_stat case for consistency. * tests/find/inode-zero.sh: Add test. * tests/local.mk (all_tests): Reference it. Problem introduced by the inum optimisation in commit 2bf001636e6. Reported by Andrea Monaco <andrea.mon...@autistici.org> in https://lists.gnu.org/r/bug-findutils/2021-12/msg00008.html --- NEWS | 5 ++++ find/defs.h | 1 - find/pred.c | 2 -- find/util.c | 31 ++++------------------- tests/find/inode-zero.sh | 54 ++++++++++++++++++++++++++++++++++++++++ tests/local.mk | 1 + 6 files changed, 65 insertions(+), 29 deletions(-) create mode 100755 tests/find/inode-zero.sh diff --git a/NEWS b/NEWS index 046eaddb..ffde4720 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,11 @@ GNU findutils NEWS - User visible changes. -*- outline -*- (allout) if the -L option was not given. [Bug present since the SELinux implementation in 4.5.6] + 'find -inum' and 'find -printf %i' now also work on platforms which allow + the inode number Zero; e.g. the GNU/Hurd uses inode number 0 for /dev/console. + Previously, find(1) would abort when visiting such a file. + [Bug present since FINDUTILS_4_5_4-1.] + ** Documentation Changes The find.1 man page and the Texinfo manual now show environment variables diff --git a/find/defs.h b/find/defs.h index cb519136..63ab7eda 100644 --- a/find/defs.h +++ b/find/defs.h @@ -519,7 +519,6 @@ bool apply_predicate(const char *pathname, struct stat *stat_buf, struct predica /* util.c. */ -int get_info (const char *pathname, struct stat *p, struct predicate *pred_ptr); bool following_links (void); bool digest_mode (mode_t *mode, const char *pathname, const char *name, struct stat *pstat, bool leaf); bool default_prints (struct predicate *pred); diff --git a/find/pred.c b/find/pred.c index bf995723..338ac14a 100644 --- a/find/pred.c +++ b/find/pred.c @@ -498,8 +498,6 @@ pred_inum (const char *pathname, struct stat *stat_buf, struct predicate *pred_p { (void) pathname; - assert (stat_buf->st_ino != 0); - switch (pred_ptr->args.numinfo.kind) { case COMP_GT: diff --git a/find/util.c b/find/util.c index afd9880e..59f80659 100644 --- a/find/util.c +++ b/find/util.c @@ -280,7 +280,7 @@ get_statinfo (const char *pathname, const char *name, struct stat *p) /* Get the stat/type/inode information for a file, if it is not * already known. Returns 0 on success (or if we did nothing). */ -int +static int get_info (const char *pathname, struct stat *p, struct predicate *pred_ptr) @@ -290,7 +290,7 @@ get_info (const char *pathname, /* If we need the full stat info, or we need the type info but don't * already have it, stat the file now. */ - if (pred_ptr->need_stat) + if (pred_ptr->need_stat && !state.have_stat) { todo = true; /* need full stat info */ } @@ -316,31 +316,10 @@ get_info (const char *pathname, } if (todo) { - int result = get_statinfo (pathname, state.rel_pathname, p); - if (result != 0) - { - return -1; /* failure. */ - } - else - { - /* Verify some postconditions. We can't check st_mode for - non-zero-ness because of Savannah bug #16378 (which is - that broken NFS servers can return st_mode==0). */ - if (pred_ptr->need_type) - { - assert (state.have_type); - } - if (pred_ptr->need_inum) - { - assert (p->st_ino); - } - return 0; /* success. */ - } - } - else - { - return 0; /* success; nothing to do. */ + if (get_statinfo (pathname, state.rel_pathname, p) != 0) + return -1; /* failure. */ } + return 0; /* success, or nothing to do. */ } /* Determine if we can use O_NOFOLLOW. diff --git a/tests/find/inode-zero.sh b/tests/find/inode-zero.sh new file mode 100755 index 00000000..7bfc2698 --- /dev/null +++ b/tests/find/inode-zero.sh @@ -0,0 +1,54 @@ +#!/bin/sh +# Ensure find(1) treats inode number 0 correctly. + +# Copyright (C) 2021 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 <https://www.gnu.org/licenses/>. + +. "${srcdir=.}/tests/init.sh"; fu_path_prepend_ +print_ver_ find + +# Skip test unless we find a file with inode number 0. +# GNU/Hurd uses inode 0 for /dev/console. +f='/dev/console' +test -e "${f}" \ + && ino=$( stat -c '%i' "${f}" ) \ + && test "${ino}" = '0' \ + || skip_ "no file with inode number 0 here" + +echo "${f}" > exp || framework_failure_ + +# Ensure -inum works. +# Find by exact inode number 0. +find "${f}" -inum 0 >out 2>err || fail=1 +compare exp out || fail=1 +compare /dev/null err || fail=1 + +# Find by inode number <1. +find "${f}" -inum -1 >out 2>err || fail=1 +compare exp out || fail=1 +compare /dev/null err || fail=1 + +# No match with unrelated inode number. +find "${f}" -inum 12345 >out 2>err || fail=1 +compare /dev/null out || fail=1 +compare /dev/null err || fail=1 + +# Ensure '-printf "%i"' works. +echo 0 > exp || framework_failure_ +find "${f}" -printf '%i\n' >out 2>err || fail=1 +compare exp out || fail=1 +compare /dev/null err || fail=1 + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index 17efc591..5ddcaaf9 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -107,6 +107,7 @@ check-root: all_tests = \ tests/misc/help-version.sh \ tests/find/depth-unreadable-dir.sh \ + tests/find/inode-zero.sh \ tests/find/many-dir-entries-vs-OOM.sh \ tests/find/name-lbracket-literal.sh \ tests/find/printf_escapechars.sh \ -- 2.34.1