All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] generic/062: filter setfattr --restore symlink-safety warning
@ 2026-07-06  6:11 Avinesh Kumar
  2026-07-06 19:13 ` [PATCH v2] " Avinesh Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: Avinesh Kumar @ 2026-07-06  6:11 UTC (permalink / raw)
  To: fstests; +Cc: zlang

From: Avinesh Kumar <avinesh.kumar@suse.com>

attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" warn when
used without -P and the dump contains a symlink, which generic/062 does.
The warning leaks into the compared output via the setfattr() wrapper
and fails the test. Adding -P isn't portable (older attr doesn't have it),
so filter the warning in the wrapper instead.

[0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8

Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
---
 tests/generic/062 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/generic/062 b/tests/generic/062
index 89659040..9ccfde36 100755
--- a/tests/generic/062
+++ b/tests/generic/062
@@ -30,7 +30,13 @@ getfattr()
 
 setfattr()
 {
-    $SETFATTR_PROG $@ 2>&1 | _filter_scratch
+	# attr >= 2.6.0 (CVE-2026-54371 fix) warns that "setfattr --restore" without
+	# -P/--physical is unsafe because it can traverse symlinks. Older attr does
+	# not accept -P, so just filter the warning to stay version-agnostic.
+	$SETFATTR_PROG $@ 2>&1 | \
+	sed -e '/^setfattr: .*--restore.* is unsafe without option/d' \
+	    -e '/^Warning: option --restore=.* is unsafe without option/d' | \
+	_filter_scratch
 }
 
 _create_test_bed()
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2] generic/062: filter setfattr --restore symlink-safety warning
  2026-07-06  6:11 [PATCH] generic/062: filter setfattr --restore symlink-safety warning Avinesh Kumar
@ 2026-07-06 19:13 ` Avinesh Kumar
  2026-07-17  9:22   ` Zorro Lang
  0 siblings, 1 reply; 5+ messages in thread
From: Avinesh Kumar @ 2026-07-06 19:13 UTC (permalink / raw)
  To: fstests; +Cc: zlang

From: Avinesh Kumar <avinesh.kumar@suse.com>

attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" warn when
used without -P and the dump contains a symlink, which generic/062 does.
The warning leaks into the compared output via the setfattr() wrapper
and fails the test. Adding -P isn't portable (older attr doesn't have it),
so filter the warning in the wrapper instead.

[0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8

Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
---
 tests/generic/062 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/generic/062 b/tests/generic/062
index 89659040..ddf0a478 100755
--- a/tests/generic/062
+++ b/tests/generic/062
@@ -30,7 +30,12 @@ getfattr()
 
 setfattr()
 {
-    $SETFATTR_PROG $@ 2>&1 | _filter_scratch
+    # attr >= 2.6.0 (CVE-2026-54371 fix) warns that "setfattr --restore" without
+    # -P/--physical is unsafe because it can traverse symlinks. Older attr does
+    # not accept -P, so just filter the warning to stay version-agnostic.
+    $SETFATTR_PROG $@ 2>&1 | \
+        sed -e '/^Warning: option --restore=file is unsafe without option/d' | \
+        _filter_scratch
 }
 
 _create_test_bed()
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] generic/062: filter setfattr --restore symlink-safety warning
  2026-07-06 19:13 ` [PATCH v2] " Avinesh Kumar
@ 2026-07-17  9:22   ` Zorro Lang
  2026-07-17 12:04     ` Avinesh Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: Zorro Lang @ 2026-07-17  9:22 UTC (permalink / raw)
  To: Avinesh Kumar; +Cc: fstests

On Mon, Jul 06, 2026 at 09:13:26PM +0200, Avinesh Kumar wrote:
> From: Avinesh Kumar <avinesh.kumar@suse.com>
> 
> attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" warn when
> used without -P and the dump contains a symlink, which generic/062 does.
> The warning leaks into the compared output via the setfattr() wrapper
> and fails the test. Adding -P isn't portable (older attr doesn't have it),
> so filter the warning in the wrapper instead.
> 
> [0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8

For generic/062, modifying the g/062 itself seems fine and logical, as
_extend_test_bed() explicitly introduces complex symlink paths for
testing purposes.

Have you run xfstests on XFS with attr 2.6.0? There are quite a few
setfattr --restore invocations in common/populate. If those also trigger
the new unsafe restore warnings, we might want to consider introducing
a helper function, such as _setfattr or _setfattr_restore to handle to
-h and -P things.

Reviewed-by: Zorro Lang <zlang@kernel.org>

> 
> Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
> ---
>  tests/generic/062 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/generic/062 b/tests/generic/062
> index 89659040..ddf0a478 100755
> --- a/tests/generic/062
> +++ b/tests/generic/062
> @@ -30,7 +30,12 @@ getfattr()
>  
>  setfattr()
>  {
> -    $SETFATTR_PROG $@ 2>&1 | _filter_scratch
> +    # attr >= 2.6.0 (CVE-2026-54371 fix) warns that "setfattr --restore" without
> +    # -P/--physical is unsafe because it can traverse symlinks. Older attr does
> +    # not accept -P, so just filter the warning to stay version-agnostic.
> +    $SETFATTR_PROG $@ 2>&1 | \
> +        sed -e '/^Warning: option --restore=file is unsafe without option/d' | \
> +        _filter_scratch
>  }
>  
>  _create_test_bed()
> -- 
> 2.54.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] generic/062: filter setfattr --restore symlink-safety warning
  2026-07-17  9:22   ` Zorro Lang
@ 2026-07-17 12:04     ` Avinesh Kumar
  2026-07-17 17:10       ` Zorro Lang
  0 siblings, 1 reply; 5+ messages in thread
From: Avinesh Kumar @ 2026-07-17 12:04 UTC (permalink / raw)
  To: fstests

Hello Zorro,

Thanks for the review.

On 7/17/26 11:22 AM, Zorro Lang wrote:
> On Mon, Jul 06, 2026 at 09:13:26PM +0200, Avinesh Kumar wrote:
>> From: Avinesh Kumar <avinesh.kumar@suse.com>
>>
>> attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" warn when
>> used without -P and the dump contains a symlink, which generic/062 does.
>> The warning leaks into the compared output via the setfattr() wrapper
>> and fails the test. Adding -P isn't portable (older attr doesn't have it),
>> so filter the warning in the wrapper instead.
>>
>> [0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
> 
> For generic/062, modifying the g/062 itself seems fine and logical, as
> _extend_test_bed() explicitly introduces complex symlink paths for
> testing purposes.
> 
> Have you run xfstests on XFS with attr 2.6.0? There are quite a few
> setfattr --restore invocations in common/populate. If those also trigger
> the new unsafe restore warnings, we might want to consider introducing
> a helper function, such as _setfattr or _setfattr_restore to handle to
> -h and -P things.
> 
> Reviewed-by: Zorro Lang <zlang@kernel.org>

I am sorry, I forgot to reply to my patch here. I got same feedback from
Darrick when I sent patch for xfs/083 for the same issue. So this is
being handled with a wrapper. Please review here and this patch can
be ignored -
https://lore.kernel.org/fstests/9b95ab03-34c0-4d2f-b4d7-fdb9c8cbb193@suse.com/T/#m3bef5e49d3ca17f845a3ccab8b6dea5b67c00d86


Regards,
Avinesh

> 
>>
>> Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
>> ---
>>   tests/generic/062 | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/generic/062 b/tests/generic/062
>> index 89659040..ddf0a478 100755
>> --- a/tests/generic/062
>> +++ b/tests/generic/062
>> @@ -30,7 +30,12 @@ getfattr()
>>   
>>   setfattr()
>>   {
>> -    $SETFATTR_PROG $@ 2>&1 | _filter_scratch
>> +    # attr >= 2.6.0 (CVE-2026-54371 fix) warns that "setfattr --restore" without
>> +    # -P/--physical is unsafe because it can traverse symlinks. Older attr does
>> +    # not accept -P, so just filter the warning to stay version-agnostic.
>> +    $SETFATTR_PROG $@ 2>&1 | \
>> +        sed -e '/^Warning: option --restore=file is unsafe without option/d' | \
>> +        _filter_scratch
>>   }
>>   
>>   _create_test_bed()
>> -- 
>> 2.54.0
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] generic/062: filter setfattr --restore symlink-safety warning
  2026-07-17 12:04     ` Avinesh Kumar
@ 2026-07-17 17:10       ` Zorro Lang
  0 siblings, 0 replies; 5+ messages in thread
From: Zorro Lang @ 2026-07-17 17:10 UTC (permalink / raw)
  To: Avinesh Kumar; +Cc: fstests

On Fri, Jul 17, 2026 at 02:04:20PM +0200, Avinesh Kumar wrote:
> Hello Zorro,
> 
> Thanks for the review.
> 
> On 7/17/26 11:22 AM, Zorro Lang wrote:
> > On Mon, Jul 06, 2026 at 09:13:26PM +0200, Avinesh Kumar wrote:
> > > From: Avinesh Kumar <avinesh.kumar@suse.com>
> > > 
> > > attr 2.6.0 (CVE-2026-54371 fix)[0] makes "setfattr --restore" warn when
> > > used without -P and the dump contains a symlink, which generic/062 does.
> > > The warning leaks into the compared output via the setfattr() wrapper
> > > and fails the test. Adding -P isn't portable (older attr doesn't have it),
> > > so filter the warning in the wrapper instead.
> > > 
> > > [0] https://cgit.git.savannah.nongnu.org/cgit/attr.git/commit/?id=3fb06b9ba314d37035d0877e6de313de754f1ac8
> > 
> > For generic/062, modifying the g/062 itself seems fine and logical, as
> > _extend_test_bed() explicitly introduces complex symlink paths for
> > testing purposes.
> > 
> > Have you run xfstests on XFS with attr 2.6.0? There are quite a few
> > setfattr --restore invocations in common/populate. If those also trigger
> > the new unsafe restore warnings, we might want to consider introducing
> > a helper function, such as _setfattr or _setfattr_restore to handle to
> > -h and -P things.
> > 
> > Reviewed-by: Zorro Lang <zlang@kernel.org>
> 
> I am sorry, I forgot to reply to my patch here. I got same feedback from
> Darrick when I sent patch for xfs/083 for the same issue. So this is
> being handled with a wrapper. Please review here and this patch can
> be ignored -
> https://lore.kernel.org/fstests/9b95ab03-34c0-4d2f-b4d7-fdb9c8cbb193@suse.com/T/#m3bef5e49d3ca17f845a3ccab8b6dea5b67c00d86

Great! Darrick gave you a similar review point, I'm going to check that new
patch.

Thanks,
Zorro

> 
> 
> Regards,
> Avinesh
> 
> > 
> > > 
> > > Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>
> > > ---
> > >   tests/generic/062 | 7 ++++++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/generic/062 b/tests/generic/062
> > > index 89659040..ddf0a478 100755
> > > --- a/tests/generic/062
> > > +++ b/tests/generic/062
> > > @@ -30,7 +30,12 @@ getfattr()
> > >   setfattr()
> > >   {
> > > -    $SETFATTR_PROG $@ 2>&1 | _filter_scratch
> > > +    # attr >= 2.6.0 (CVE-2026-54371 fix) warns that "setfattr --restore" without
> > > +    # -P/--physical is unsafe because it can traverse symlinks. Older attr does
> > > +    # not accept -P, so just filter the warning to stay version-agnostic.
> > > +    $SETFATTR_PROG $@ 2>&1 | \
> > > +        sed -e '/^Warning: option --restore=file is unsafe without option/d' | \
> > > +        _filter_scratch
> > >   }
> > >   _create_test_bed()
> > > -- 
> > > 2.54.0
> > > 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-17 17:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06  6:11 [PATCH] generic/062: filter setfattr --restore symlink-safety warning Avinesh Kumar
2026-07-06 19:13 ` [PATCH v2] " Avinesh Kumar
2026-07-17  9:22   ` Zorro Lang
2026-07-17 12:04     ` Avinesh Kumar
2026-07-17 17:10       ` Zorro Lang

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.