On 10/03/2025 09:11, Frédéric Yhuel wrote:
On 3/9/25 20:22, Pádraig Brady wrote:
On 09/03/2025 17:35, Frédéric Yhuel via GNU coreutils General Discussion
wrote:
Hi,
dd doesn't error out if posix_fadvise() returns an error. Here is a
patch for the fix.
fadvise64() always returns -1 on error, but not posix_fadvise().
Good catch.
The main issue of processing non seekable files was
already diagnosed with other operations related to the "nocache" feature,
OK
but this may diagnose other edge cases.
One of these (very unlikely) cases would be syscall filtering.
For example, using systemd, a configuration like this one:
SystemCallFilter=~fadvise64
SystemCallErrorNumber=EPERM
I've adjusted your patch (attached) to
simplify a bit, always set errno, and add a NEWS entry.
Thanks! FWIW, I have tested your new patch, and it works well (better).
OK I've pushed your patch.
I also pushed the attached test in addition.
cheers,
Pádraig
From 2b0887fdd507337c58ba964c283d73336a8da2cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 10 Mar 2025 12:33:20 +0000
Subject: [PATCH] tests: dd: ensure posix_fadvise errors are handled
* tests/dd/nocache_fail.sh: Add a test case for the recent fix.
---
tests/dd/nocache_fail.sh | 56 ++++++++++++++++++++++++++++++++++++++++
tests/local.mk | 1 +
2 files changed, 57 insertions(+)
create mode 100755 tests/dd/nocache_fail.sh
diff --git a/tests/dd/nocache_fail.sh b/tests/dd/nocache_fail.sh
new file mode 100755
index 000000000..4d30c8558
--- /dev/null
+++ b/tests/dd/nocache_fail.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+# Ensure we diagnose failure to drop caches
+# We didn't check the return from posix_fadvise() correctly before v9.7
+
+# Copyright (C) 2025 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"; path_prepend_ ./src
+print_ver_ dd
+require_gcc_shared_
+
+# Replace each getxattr and lgetxattr call with a call to these stubs.
+# Count those and write the total number of calls to the file "x"
+# via a global destructor.
+cat > k.c <<'EOF' || framework_failure_
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+int posix_fadvise (int fd, off_t offset, off_t len, int advice)
+{
+ fopen ("called", "w");
+ return ENOTSUP; /* Simulate non standard error indication. */
+}
+EOF
+
+# Then compile/link it:
+gcc_shared_ k.c k.so \
+ || framework_failure_ 'failed to build shared library'
+
+touch ifile || framework_failure_
+
+LD_PRELOAD=$LD_PRELOAD:./k.so dd if=ifile iflag=nocache count=0 2>err
+ret=$?
+
+test -f called || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
+
+grep 'dd: failed to discard cache for: ifile' err || fail=1
+
+# Ensure that the dd command failed
+test "$ret" = 1 || fail=1
+
+Exit $fail
diff --git a/tests/local.mk b/tests/local.mk
index 12e30b449..4da6756ac 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -553,6 +553,7 @@ all_tests = \
tests/dd/no-allocate.sh \
tests/dd/nocache.sh \
tests/dd/nocache_eof.sh \
+ tests/dd/nocache_fail.sh \
tests/dd/not-rewound.sh \
tests/dd/reblock.sh \
tests/dd/skip-seek.pl \
--
2.48.1
