* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.