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
