[Bug 215687] New: chown behavior on XFS is changed

[Bug 215687] New: chown behavior on XFS is changed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215687

            Bug ID: 215687
           Summary: chown behavior on XFS is changed
           Product: File System
           Version: 2.5
    Kernel Version: xfs-5.18-merge-1
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: XFS
          Assignee: filesystem_xfs@kernel-bugs.kernel.org
          Reporter: zlang@redhat.com
        Regression: No

From our regular regression test on XFS, we found a failure which never failed
before:
# prove -rf /root/git/pjd-fstest/tests/chown/00.t 
/root/git/pjd-fstest/tests/chown/00.t .. 83/171 
not ok 84
not ok 88
/root/git/pjd-fstest/tests/chown/00.t .. Failed 2/171 subtests 

Test Summary Report
-------------------
/root/git/pjd-fstest/tests/chown/00.t (Wstat: 0 Tests: 171 Failed: 2)
  Failed tests:  84, 88
Files=1, Tests=171, 31 wallclock secs ( 0.10 usr  0.04 sys +  1.04 cusr 15.50
csys = 16.68 CPU)
Result: FAIL

That means chown behavior has been changed on XFS. To reproduce this failure
you can:
1) mkfs.xfs -f /dev/sdb1
2) mount /dev/sdb1 /mnt/test
3) cd /mnt/test
4) git clone git://git.code.sf.net/p/ntfs-3g/pjd-fstest
5) cd pjd-fstest; make;
6) echo -e "os=Linux\nfs=xfs" > tests/conf
7) cd /mnt/test
8) prove -rf /path/to/pjd-fstest/tests/chown/00.t

I'm going to look into it, to found out what specified behavior is changed.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215687] chown behavior on XFS is changed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215687

--- Comment #1 from Zorro Lang (zlang@redhat.com) ---
Oh, looks like the failures come from below testing:

expect 0 mkdir ${n0} 0755
expect 0 chown ${n0} 65534 65533
expect 0 chmod ${n0} 06555
expect 06555 lstat ${n0} mode
expect 0 -u 65534 -g 65533,65532 chown ${n0} 65534 65532
case "${os}:${fs}" in
Linux:glusterfs)
        expect "0555,65534,65532|06555,65534,65532" lstat ${n0} mode,uid,gid
        ;;
Linux:xfs)
        expect 0555,65534,65532 lstat ${n0} mode,uid,gid
        ;;
Linux:*)
        expect 06555,65534,65532 lstat ${n0} mode,uid,gid
        ;;
*)
        expect 0555,65534,65532 lstat ${n0} mode,uid,gid
        ;;
esac

If a directory has S_ISUID(04000) and S_ISGID(02000) permission bits, XFS will
lost these 2 bits after chown the group of the directory. But now it keeps
these two bits:

# mkdir dir
# chown 65534:65533 dir
# chmod 06555 dir
# ls -ld dir
dr-sr-sr-x. 2 nobody 65533 6 Mar 15 16:26 dir
# chown 65534:65532 dir
# ls -ld dir
dr-sr-sr-x. 2 nobody 65532 6 Mar 15 16:26 dir


Hmm... please help to make sure if this's an expected behavior change, or an
unexpected regression? Thanks.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215687] chown behavior on XFS is changed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215687

--- Comment #2 from Zorro Lang (zlang@redhat.com) ---
OK, by looking into xfs commits, I feel this patch brings in this behavior
change:

commit e014f37db1a2d109afa750042ac4d69cf3e3d88e
Author: Darrick J. Wong <djwong@kernel.org>
Date:   Tue Mar 8 10:51:16 2022 -0800

    xfs: use setattr_copy to set vfs inode attributes

We treated it as an XFS behavior ~10 years, now it's changed ...

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [Bug 215687] New: chown behavior on XFS is changed

[Thorsten Leemhuis]

Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.

On 15.03.22 09:12, bugzilla-daemon@kernel.org wrote:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215687
> 
>            Summary: chown behavior on XFS is changed

Darrick, what's up with this bug reported more than ten days ago? It's a
a regression reported the reporter even bisected to a change of yours
(e014f37db1a2 ("xfs: use setattr_copy to set vfs inode attributes") --
see the ticket for details) – but nothing happened afaics. Did the
discussion about this continue somewhere else or did it fall through the
cracks?

Anyway: I'm adding it to regzbot, my Linux kernel regression tracking bot:

#regzbot ^introduced e014f37db1a2d109afa750042ac4d69cf3e3d88e
#regzbot title xfs: chown behavior changed
#regzbot link: https://bugzilla.kernel.org/show_bug.cgi?id=215687
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.

Re: [Bug 215687] New: chown behavior on XFS is changed

[Thorsten Leemhuis]

On 24.03.22 11:22, Thorsten Leemhuis wrote:
> On 15.03.22 09:12, bugzilla-daemon@kernel.org wrote:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=215687
>>
>>            Summary: chown behavior on XFS is changed
> 
> Darrick, what's up with this bug reported more than ten days ago? 

Ahh, Zorro Lang replied in the ticket, seems things are okay now after
your discussed this on IRC, so let me remove this from the regression
tracking; if somebody really complains we ca start to track this again.

#regzbot invalid: seems reporter is okay with the changed behavior after
discussing this with the maintainer
https://bugzilla.kernel.org/show_bug.cgi?id=215687#c4

[Bug 215687] chown behavior on XFS is changed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215687

--- Comment #3 from The Linux kernel's regression tracker (Thorsten Leemhuis) (regressions@leemhuis.info) ---
Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.

On 15.03.22 09:12, bugzilla-daemon@kernel.org wrote:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215687
> 
>            Summary: chown behavior on XFS is changed

Darrick, what's up with this bug reported more than ten days ago? It's a
a regression reported the reporter even bisected to a change of yours
(e014f37db1a2 ("xfs: use setattr_copy to set vfs inode attributes") --
see the ticket for details) – but nothing happened afaics. Did the
discussion about this continue somewhere else or did it fall through the
cracks?

Anyway: I'm adding it to regzbot, my Linux kernel regression tracking bot:

#regzbot ^introduced e014f37db1a2d109afa750042ac4d69cf3e3d88e
#regzbot title xfs: chown behavior changed
#regzbot link: https://bugzilla.kernel.org/show_bug.cgi?id=215687
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215687] chown behavior on XFS is changed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215687

--- Comment #4 from Zorro Lang (zlang@redhat.com) ---
(In reply to The Linux kernel's regression tracker (Thorsten Leemhuis) from
comment #3)
> Hi, this is your Linux kernel regression tracker. Top-posting for once,
> to make this easily accessible to everyone.
> 
> On 15.03.22 09:12, bugzilla-daemon@kernel.org wrote:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=215687
> > 
> >            Summary: chown behavior on XFS is changed
> 
> Darrick, what's up with this bug reported more than ten days ago? It's a
> a regression reported the reporter even bisected to a change of yours
> (e014f37db1a2 ("xfs: use setattr_copy to set vfs inode attributes") --
> see the ticket for details) – but nothing happened afaics. Did the
> discussion about this continue somewhere else or did it fall through the
> cracks?
> 
> Anyway: I'm adding it to regzbot, my Linux kernel regression tracking bot:
> 
> #regzbot ^introduced e014f37db1a2d109afa750042ac4d69cf3e3d88e
> #regzbot title xfs: chown behavior changed
> #regzbot link: https://bugzilla.kernel.org/show_bug.cgi?id=215687
> #regzbot ignore-activity
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.

Darrick has talked about it with us in IRC (as below, hope that helps):

2022-03-16 00:02 < djwong> all that setgid dropping came out of complaints that
xfs didn't handle that the same way as all the other linux filesystems
2022-03-16 00:03 < djwong> zlang: ^^^
2022-03-16 00:04 < djwong> originally the xfs setattr more or less did what the
vfs setattr did
2022-03-16 00:04 < djwong> but now people update the vfs setattr and they don't
update the xfs version
2022-03-16 00:05 < djwong> so is this a "unique feature of xfs"?
2022-03-16 00:06 < djwong> inconsistent behavior from xfs?
2022-03-16 00:06 < djwong> or just bitrotting crap in the kernel?
2022-03-16 01:46 < zlang> djwong, sandeen: Thanks! I don't know if there's a
standard describe that, just thought about how should we backport it, hope no
customer depend on the old behavior :)
2022-03-16 01:55 < zlang> That would be great if no customer depend on that, or
they might complain, if their script expect a program lose S_ISUID and S_ISGID
after chown, but not, then cause permission/security problem
2022-03-16 02:00 < zlang> So if we backport that, we might be better to warn
that in doc. To remind them if they hope to "lose" S_ISUID and S_ISGID bits,
better to do that clearly and definitely
2022-03-16 02:01 < djwong> <nod> all that setgid handling is ... very murky
2022-03-16 02:01 < djwong> it at least matches ext4 and btrfs now :P


And another bug report (which can be closed DUP on this one):
https://bugzilla.kernel.org/show_bug.cgi?id=215693

Darrick has reviewed and replied in IRC (update as below):

2022-03-16 17:10 < zlang> djwong: Did you notice that generic/673 fails on
xfs-5.18-merge-1, looks similar with that chown problem
2022-03-16 17:30 < zlang> https://bugzilla.kernel.org/show_bug.cgi?id=215693
2022-03-16 17:32 < zlang> But this's about reflink (not chown), and sometimes
it lose sgid bit after reflink, sometimes not ...
2022-03-16 17:39 < zlang> So I report a seperate bug to track this question,
please help to review and make sure the new expected behaviors. Sorry to bring
this trouble to you
2022-03-17 00:36 < djwong> zlang: both setgid changes that you filed bugs
against stem from the same setattr_copy issue
2022-03-17 00:37 < djwong> also generic/673 is wrong, see
https://lore.kernel.org/fstests/164740142591.3371628.12793589713189041823.stgit@magnolia/T/#u

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215687] chown behavior on XFS is changed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215687

--- Comment #5 from The Linux kernel's regression tracker (Thorsten Leemhuis) (regressions@leemhuis.info) ---
On 24.03.22 11:22, Thorsten Leemhuis wrote:
> On 15.03.22 09:12, bugzilla-daemon@kernel.org wrote:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=215687
>>
>>            Summary: chown behavior on XFS is changed
> 
> Darrick, what's up with this bug reported more than ten days ago? 

Ahh, Zorro Lang replied in the ticket, seems things are okay now after
your discussed this on IRC, so let me remove this from the regression
tracking; if somebody really complains we ca start to track this again.

#regzbot invalid: seems reporter is okay with the changed behavior after
discussing this with the maintainer
https://bugzilla.kernel.org/show_bug.cgi?id=215687#c4

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.