* [PATCH] xfs/083: redirect populate stderr to avoid spurious attr restore warnings
@ 2026-07-10 19:53 Avinesh Kumar
2026-07-10 23:46 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Avinesh Kumar @ 2026-07-10 19:53 UTC (permalink / raw)
To: fstests; +Cc: zlang, djwong
From: Avinesh Kumar <avinesh.kumar@suse.com>
attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" print warnings
on stderr. _scratch_populate uses it, and xfs/083 only redirects stdout,
so the warnings leak into the test output and fail it.
Redirect stderr to $seqres.full too.
+Warning: option --restore=file is unsafe without option -P (--physical) as it traverses symbolic links in pathnames
+Warning: option --restore=file is unsafe without option -h (--no-dereference) as it dereferences symbolic link pathnames
[0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
---
tests/xfs/083 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/xfs/083 b/tests/xfs/083
index 9291c8c0..48deaafa 100755
--- a/tests/xfs/083
+++ b/tests/xfs/083
@@ -91,7 +91,7 @@ echo "+ create scratch fs" >> $seqres.full
_scratch_mkfs_xfs >> $seqres.full 2>&1
echo "+ populate fs image" >> $seqres.full
-_scratch_populate >> $seqres.full
+_scratch_populate >> $seqres.full 2>&1
echo "+ check fs" >> $seqres.full
_repair_scratch_fs >> $seqres.full 2>&1 || _fail "should pass initial fsck"
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs/083: redirect populate stderr to avoid spurious attr restore warnings
2026-07-10 19:53 [PATCH] xfs/083: redirect populate stderr to avoid spurious attr restore warnings Avinesh Kumar
@ 2026-07-10 23:46 ` Darrick J. Wong
2026-07-13 13:52 ` [PATCH v2] common: strip attr 2.6.0 --restore safety warnings Avinesh Kumar
0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2026-07-10 23:46 UTC (permalink / raw)
To: Avinesh Kumar; +Cc: fstests, zlang
On Fri, Jul 10, 2026 at 09:53:55PM +0200, Avinesh Kumar wrote:
> From: Avinesh Kumar <avinesh.kumar@suse.com>
>
> attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" print warnings
> on stderr. _scratch_populate uses it, and xfs/083 only redirects stdout,
> so the warnings leak into the test output and fail it.
> Redirect stderr to $seqres.full too.
>
> +Warning: option --restore=file is unsafe without option -P (--physical) as it traverses symbolic links in pathnames
> +Warning: option --restore=file is unsafe without option -h (--no-dereference) as it dereferences symbolic link pathnames
>
> [0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
>
> Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
> ---
> tests/xfs/083 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/xfs/083 b/tests/xfs/083
> index 9291c8c0..48deaafa 100755
> --- a/tests/xfs/083
> +++ b/tests/xfs/083
> @@ -91,7 +91,7 @@ echo "+ create scratch fs" >> $seqres.full
> _scratch_mkfs_xfs >> $seqres.full 2>&1
>
> echo "+ populate fs image" >> $seqres.full
> -_scratch_populate >> $seqres.full
> +_scratch_populate >> $seqres.full 2>&1
That's quite an overbroad brush there -- we want any *unexpected*
errors to go to the .out file. Given there are five users of setfattr
--restore flag:
$ git grep -i attr.*--restore
common/overlay:497: _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | $SETFATTR_PROG --restore=-
common/populate:154: ) | setfattr --restore -
common/populate:203: ) | setfattr --restore -
tests/generic/062:189:setfattr -h --restore=$tmp.backup1
tests/xfs/227:75: $SETFATTR_PROG --restore=$tmp.$1.attrs $2
Maaaaybe this should instead be a targeted wrapper for that?
_setfattr_restore()
{
$SETFATTR_PROG --restore=- 2> $tmp.setfattr.$$
local ret=$?
cat $tmp.setfattr.$$ | sed -e '/--restore=.*unsafe.*without/d' 1>&2
return $ret
}
--D
>
> echo "+ check fs" >> $seqres.full
> _repair_scratch_fs >> $seqres.full 2>&1 || _fail "should pass initial fsck"
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] common: strip attr 2.6.0 --restore safety warnings
2026-07-10 23:46 ` Darrick J. Wong
@ 2026-07-13 13:52 ` Avinesh Kumar
2026-07-13 16:14 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Avinesh Kumar @ 2026-07-13 13:52 UTC (permalink / raw)
To: djwong; +Cc: avinesh.kumar, fstests, zlang
From: Avinesh Kumar <avinesh.kumar@suse.com>
attr 2.6.0 (CVE-2026-54371 hardening) makes "setfattr --restore" print a
warning to stderr when neither -P nor -h is given, claiming it may
traverse or dereference symlinks in the dump's pathnames [0]. This warning
leaks into test output and breaks generic/062 and xfs/083 (via
_scratch_populate).
Add a _setfattr_restore helper that runs "setfattr --restore", filters
out just the expected warning line, and preserves any other (unexpected)
stderr. Convert the callers in common/populate, common/overlay and
generic/062 to use it.
[0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
---
common/overlay | 2 +-
common/populate | 4 ++--
common/rc | 8 ++++++++
tests/generic/062 | 2 +-
4 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/common/overlay b/common/overlay
index d32f3219..73841dd7 100644
--- a/common/overlay
+++ b/common/overlay
@@ -494,7 +494,7 @@ _overlay_trusted_to_user()
local dir=$1
for file in `find $dir`; do
- _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | $SETFATTR_PROG --restore=-
+ _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | _setfattr_restore --restore=-
for xattr in `_getfattr --absolute-names -d -m '^trusted.overlay.' $file | tail -n +2 | cut -d= -f1`; do
$SETFATTR_PROG -x $xattr $file;
done
diff --git a/common/populate b/common/populate
index 1c0dd03e..f7fcd0c3 100644
--- a/common/populate
+++ b/common/populate
@@ -151,7 +151,7 @@ __populate_create_attr() {
echo "# file: ${name}";
seq --format "user.%08g=\"abcdefgh\"" 0 "${nr}"
echo
- ) | setfattr --restore -
+ ) | _setfattr_restore --restore=-
test -z "${missing}" && return
seq 1 2 "${nr}" | while read d; do
@@ -200,7 +200,7 @@ __populate_xfs_create_btree_attr() {
seq --format "user.%08g=\"abcdefgh\"" "${nr}" "$((nr + incr + 1))"
echo "user.v$(printf "%.08d" "$nr")=\"${bigval}\""
echo
- ) | setfattr --restore -
+ ) | _setfattr_restore --restore=-
done
# ... and in the second loop we delete all the remote attrs to
diff --git a/common/rc b/common/rc
index 79189e7e..6234a580 100644
--- a/common/rc
+++ b/common/rc
@@ -132,6 +132,14 @@ _test_fsxattr_xflag()
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
}
+_setfattr_restore()
+{
+ $SETFATTR_PROG "$@" 2> $tmp.setfattr.$$
+ local ret=$?
+ cat $tmp.setfattr.$$ | sed -e '/--restore=.*unsafe.*without/d' 1>&2
+ return $ret
+}
+
# This test requires extsize support on the filesystem
_require_scratch_extsize()
{
diff --git a/tests/generic/062 b/tests/generic/062
index 89659040..3ef96da1 100755
--- a/tests/generic/062
+++ b/tests/generic/062
@@ -186,7 +186,7 @@ _create_test_bed
_extend_test_bed
echo "*** restore everything"
-setfattr -h --restore=$tmp.backup1
+_setfattr_restore -h --restore=$tmp.backup1 2>&1 | _filter_scratch
_backup $tmp.backup2
echo "AFTER RESTORE" >>$seqres.full
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] common: strip attr 2.6.0 --restore safety warnings
2026-07-13 13:52 ` [PATCH v2] common: strip attr 2.6.0 --restore safety warnings Avinesh Kumar
@ 2026-07-13 16:14 ` Darrick J. Wong
2026-07-13 19:21 ` Avinesh Kumar
0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2026-07-13 16:14 UTC (permalink / raw)
To: Avinesh Kumar; +Cc: fstests, zlang
On Mon, Jul 13, 2026 at 03:52:56PM +0200, Avinesh Kumar wrote:
> From: Avinesh Kumar <avinesh.kumar@suse.com>
>
> attr 2.6.0 (CVE-2026-54371 hardening) makes "setfattr --restore" print a
> warning to stderr when neither -P nor -h is given, claiming it may
> traverse or dereference symlinks in the dump's pathnames [0]. This warning
> leaks into test output and breaks generic/062 and xfs/083 (via
> _scratch_populate).
>
> Add a _setfattr_restore helper that runs "setfattr --restore", filters
> out just the expected warning line, and preserves any other (unexpected)
> stderr. Convert the callers in common/populate, common/overlay and
> generic/062 to use it.
>
> [0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
> ---
> common/overlay | 2 +-
> common/populate | 4 ++--
> common/rc | 8 ++++++++
> tests/generic/062 | 2 +-
> 4 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/common/overlay b/common/overlay
> index d32f3219..73841dd7 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -494,7 +494,7 @@ _overlay_trusted_to_user()
> local dir=$1
>
> for file in `find $dir`; do
> - _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | $SETFATTR_PROG --restore=-
> + _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | _setfattr_restore --restore=-
> for xattr in `_getfattr --absolute-names -d -m '^trusted.overlay.' $file | tail -n +2 | cut -d= -f1`; do
> $SETFATTR_PROG -x $xattr $file;
> done
> diff --git a/common/populate b/common/populate
> index 1c0dd03e..f7fcd0c3 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -151,7 +151,7 @@ __populate_create_attr() {
> echo "# file: ${name}";
> seq --format "user.%08g=\"abcdefgh\"" 0 "${nr}"
> echo
> - ) | setfattr --restore -
> + ) | _setfattr_restore --restore=-
>
> test -z "${missing}" && return
> seq 1 2 "${nr}" | while read d; do
> @@ -200,7 +200,7 @@ __populate_xfs_create_btree_attr() {
> seq --format "user.%08g=\"abcdefgh\"" "${nr}" "$((nr + incr + 1))"
> echo "user.v$(printf "%.08d" "$nr")=\"${bigval}\""
> echo
> - ) | setfattr --restore -
> + ) | _setfattr_restore --restore=-
> done
>
> # ... and in the second loop we delete all the remote attrs to
> diff --git a/common/rc b/common/rc
> index 79189e7e..6234a580 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -132,6 +132,14 @@ _test_fsxattr_xflag()
> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
> }
>
> +_setfattr_restore()
> +{
> + $SETFATTR_PROG "$@" 2> $tmp.setfattr.$$
Hrm... I suppose if you're going to allow any arguments to setfattr (and
not specifically require --restore=) then the helper ought to be called
_setfattr.
Other than naming, the changes look ok to me.
--D
> + local ret=$?
> + cat $tmp.setfattr.$$ | sed -e '/--restore=.*unsafe.*without/d' 1>&2
> + return $ret
> +}
> +
> # This test requires extsize support on the filesystem
> _require_scratch_extsize()
> {
> diff --git a/tests/generic/062 b/tests/generic/062
> index 89659040..3ef96da1 100755
> --- a/tests/generic/062
> +++ b/tests/generic/062
> @@ -186,7 +186,7 @@ _create_test_bed
> _extend_test_bed
>
> echo "*** restore everything"
> -setfattr -h --restore=$tmp.backup1
> +_setfattr_restore -h --restore=$tmp.backup1 2>&1 | _filter_scratch
> _backup $tmp.backup2
>
> echo "AFTER RESTORE" >>$seqres.full
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] common: strip attr 2.6.0 --restore safety warnings
2026-07-13 16:14 ` Darrick J. Wong
@ 2026-07-13 19:21 ` Avinesh Kumar
0 siblings, 0 replies; 5+ messages in thread
From: Avinesh Kumar @ 2026-07-13 19:21 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: fstests, zlang
Hello Darrick,
Thanks for the review.
On 7/13/26 6:14 PM, Darrick J. Wong wrote:
> On Mon, Jul 13, 2026 at 03:52:56PM +0200, Avinesh Kumar wrote:
>> From: Avinesh Kumar <avinesh.kumar@suse.com>
>>
>> attr 2.6.0 (CVE-2026-54371 hardening) makes "setfattr --restore" print a
>> warning to stderr when neither -P nor -h is given, claiming it may
>> traverse or dereference symlinks in the dump's pathnames [0]. This warning
>> leaks into test output and breaks generic/062 and xfs/083 (via
>> _scratch_populate).
>>
>> Add a _setfattr_restore helper that runs "setfattr --restore", filters
>> out just the expected warning line, and preserves any other (unexpected)
>> stderr. Convert the callers in common/populate, common/overlay and
>> generic/062 to use it.
>>
>> [0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
>>
>> Suggested-by: Darrick J. Wong <djwong@kernel.org>
>> Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
>> ---
>> common/overlay | 2 +-
>> common/populate | 4 ++--
>> common/rc | 8 ++++++++
>> tests/generic/062 | 2 +-
>> 4 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/common/overlay b/common/overlay
>> index d32f3219..73841dd7 100644
>> --- a/common/overlay
>> +++ b/common/overlay
>> @@ -494,7 +494,7 @@ _overlay_trusted_to_user()
>> local dir=$1
>>
>> for file in `find $dir`; do
>> - _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | $SETFATTR_PROG --restore=-
>> + _getfattr --absolute-names -d -m '^trusted.overlay.(redirect|metacopy)$' $file | sed 's/^trusted/user/' | _setfattr_restore --restore=-
>> for xattr in `_getfattr --absolute-names -d -m '^trusted.overlay.' $file | tail -n +2 | cut -d= -f1`; do
>> $SETFATTR_PROG -x $xattr $file;
>> done
>> diff --git a/common/populate b/common/populate
>> index 1c0dd03e..f7fcd0c3 100644
>> --- a/common/populate
>> +++ b/common/populate
>> @@ -151,7 +151,7 @@ __populate_create_attr() {
>> echo "# file: ${name}";
>> seq --format "user.%08g=\"abcdefgh\"" 0 "${nr}"
>> echo
>> - ) | setfattr --restore -
>> + ) | _setfattr_restore --restore=-
>>
>> test -z "${missing}" && return
>> seq 1 2 "${nr}" | while read d; do
>> @@ -200,7 +200,7 @@ __populate_xfs_create_btree_attr() {
>> seq --format "user.%08g=\"abcdefgh\"" "${nr}" "$((nr + incr + 1))"
>> echo "user.v$(printf "%.08d" "$nr")=\"${bigval}\""
>> echo
>> - ) | setfattr --restore -
>> + ) | _setfattr_restore --restore=-
>> done
>>
>> # ... and in the second loop we delete all the remote attrs to
>> diff --git a/common/rc b/common/rc
>> index 79189e7e..6234a580 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -132,6 +132,14 @@ _test_fsxattr_xflag()
>> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
>> }
>>
>> +_setfattr_restore()
>> +{
>> + $SETFATTR_PROG "$@" 2> $tmp.setfattr.$$
>
> Hrm... I suppose if you're going to allow any arguments to setfattr (and
> not specifically require --restore=) then the helper ought to be called
> _setfattr.
>
> Other than naming, the changes look ok to me.
I made wrapper to take argument (instead of hardcoding --restore=-) to
handle the generic/062 case where it restores from a file and also have
'-h'.
And I kept helper name '_setfattr_restore' as it is still entirely
restore specific but I leave that up to you. Please let me know if I
should send revised patch.
>
> --D
>
>> + local ret=$?
>> + cat $tmp.setfattr.$$ | sed -e '/--restore=.*unsafe.*without/d' 1>&2
>> + return $ret
>> +}
>> +
>> # This test requires extsize support on the filesystem
>> _require_scratch_extsize()
>> {
>> diff --git a/tests/generic/062 b/tests/generic/062
>> index 89659040..3ef96da1 100755
>> --- a/tests/generic/062
>> +++ b/tests/generic/062
>> @@ -186,7 +186,7 @@ _create_test_bed
>> _extend_test_bed
>>
>> echo "*** restore everything"
>> -setfattr -h --restore=$tmp.backup1
>> +_setfattr_restore -h --restore=$tmp.backup1 2>&1 | _filter_scratch
>> _backup $tmp.backup2
>>
>> echo "AFTER RESTORE" >>$seqres.full
>> --
>> 2.54.0
>>
Regards,
Avinesh
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-13 19:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 19:53 [PATCH] xfs/083: redirect populate stderr to avoid spurious attr restore warnings Avinesh Kumar
2026-07-10 23:46 ` Darrick J. Wong
2026-07-13 13:52 ` [PATCH v2] common: strip attr 2.6.0 --restore safety warnings Avinesh Kumar
2026-07-13 16:14 ` Darrick J. Wong
2026-07-13 19:21 ` Avinesh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox