All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] acl updates for v6.2
@ 2022-12-12 11:19 Christian Brauner
  2022-12-13  2:56 ` Linus Torvalds
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Christian Brauner @ 2022-12-12 11:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Christian Brauner, linux-fsdevel, linux-kernel

Hey Linus,

/* Summary */
This contains the work that builds a dedicated vfs posix acl api. The origins
of this work trace back to v5.19 but it took quite a while to understand the
various filesystem specific implementations in sufficient detail and also come
up with an acceptable solution.

As we discussed and seen multiple times the current state of how posix acls are
handled isn't nice and comes with a lot of problems. For a long and detailed
explanation for just some of the issues [1] provides a good summary.

The current way of handling posix acls via the generic xattr api is error
prone, hard to maintain, and type unsafe for the vfs until we call into the
filesystem's dedicated get and set inode operations.

It is already the case that posix acls are special-cased to death all the way
through the vfs. There are an uncounted number of hacks that operate on the
uapi posix acl struct instead of the dedicated vfs struct posix_acl. And the
vfs must be involved in order to interpret and fixup posix acls before storing
them to the backing store, caching them, reporting them to userspace, or for
permission checking.

Currently a range of hacks and duct tape exist to make this work. As with most
things this is really no ones fault it's just something that happened over
time. But the code is hard to understand and difficult to maintain and one is
constantly at risk of introducing bugs and regressions when having to touch it.

Instead of continuing to hack posix acls through the xattr handlers this series
builds a dedicated posix acl api solely around the get and set inode
operations. Going forward, the vfs_get_acl(), vfs_remove_acl(), and
vfs_set_acl() helpers must be used in order to interact with posix acls. They
operate directly on the vfs internal struct posix_acl instead of abusing the
uapi posix acl struct as we currently do. In the end this removes all of the
hackiness, makes the codepaths easier to maintain, and gets us type safety.

This series passes the LTP and xfstests suites without any regressions. For
xfstests the following combinations were tested:

* xfs
* ext4
* btrfs
* overlayfs
* overlayfs on top of idmapped mounts
* orangefs
* (limited) cifs

There's more simplifications for posix acls that we can make in the future if
the basic api has made it.

A few implementation details:

* The series makes sure to retain exactly the same security and integrity module
  permission checks. See [2] for annotated callchains. Especially for the
  integrity modules this api is a win because right now they convert the uapi
  posix acl struct passed to them via a void pointer into the vfs struct
  posix_acl format to perform permission checking on the mode.

  There's a new dedicated security hook for setting posix acls which passes the
  vfs struct posix_acl not a void pointer. Basing checking on the posix acl
  stored in the uapi format is really unreliable. The vfs currently hacks around
  directly in the uapi struct storing values that frankly the security and
  integrity modules can't correctly interpret as evidenced by bugs we reported
  and fixed in this area. It's not necessarily even their fault it's just that
  the format we provide to them is sub optimal.

* Some filesystems like 9p and cifs need access to the dentry in order to get
  and set posix acls which is why they either only partially or not even at all
  implement get and set inode operations. For example, cifs allows setxattr()
  and getxattr() operations but doesn't allow permission checking based on posix
  acls because it can't implement a get acl inode operation.

  Thus, this patch series updates the set acl inode operation to take a dentry
  instead of an inode argument. However, for the get acl inode operation we
  can't do this as the old get acl method is called in e.g.,
  generic_permission() and inode_permission(). These helpers in turn are called
  in various filesystem's permission inode operation. So passing a dentry
  argument to the old get acl inode operation would amount to passing a dentry
  to the permission inode operation which we shouldn't and probably can't do.

  So instead of extending the existing inode operation Christoph suggested to
  add a new one. He also requested to ensure that the get and set acl inode
  operation taking a dentry are consistently named. So for this version the old
  get acl operation is renamed to ->get_inode_acl() and a new ->get_acl() inode
  operation taking a dentry is added. With this we can give both 9p and cifs get
  and set acl inode operations and in turn remove their complex custom posix
  xattr handlers.

  In the future I hope to get rid of the inode method duplication but it isn't
  like we have never had this situation. Readdir is just one example. And
  frankly, the overall gain in type safety and the more pleasant api wise are
  simply too big of a benefit to not accept this duplication for a while.

* We've done a full audit of every codepaths using variant of the current
  generic xattr api to get and set posix acls and surprisingly it isn't
  that many places. There's of course always a chance that we might have missed
  some and if so I'm sure we'll find them soon enough.

  The crucial codepaths to be converted are obviously stacking filesystems such
  as ecryptfs and overlayfs.

  For a list of all callers currently using generic xattr api helpers see [2]
  including comments whether they support posix acls or not.

* The old vfs generic posix acl infrastructure doesn't obey the create and
  replace semantics promised on the setxattr(2) manpage. This patch series
  doesn't address this. It really is something we should revisit later though.

The patches in this PR are roughly organized as follows:

(1) Change existing set acl inode operation to take a dentry argument.
   (Intended to be a non-functional change.)

(2) Rename existing get acl method.
    (Intended to be a non-functional change.)

(3) Implement get and set acl inode operations for filesystems that couldn't
    implement one before because of the missing dentry. That's mostly 9p and
    cifs.
    (Intended to be a non-functional change.)

(4) Build posix acl api, i.e., add vfs_get_acl(), vfs_remove_acl(), and
    vfs_set_acl() including security and integrity hooks.
    (Intended to be a non-functional change.)

(5) Implement get and set acl inode operations for stacking filesystems.
    (Intended to be a non-functional change.)

(6) Switch posix acl handling in stacking filesystems to new posix acl api now
    that all filesystems it can stack upon support it.

(7) Switch vfs to new posix acl api
    (semantical change)

(8) Remove all now unused helpers

(9) Additional regression fixes reported after we merged this into linux-next
    after v6.1-rc1:
    e40df4281b86 orangefs: fix mode handling
    5b52aebef895 ovl: call posix_acl_release() after error checking
    16257cf6658d evm: remove dead code in evm_inode_set_acl()
    cb2144d66b0b cifs: check whether acl is valid early

Thanks to Seth for a lot of good discussion around this and encouragement and
input from Christoph.

/* Testing */
clang: Ubuntu clang version 15.0.2-1
gcc: gcc (Ubuntu 12.2.0-3ubuntu1) 12.2.0

All patches are based on v6.2-rc1 and have been sitting in linux-next. No build
failures or warnings were observed. All old and new tests in fstests,
selftests, and LTP pass without regressions. But a series like this has of
course regression potential and it's almost impossible to run fstests for all
filesystems that are touched. For a bunch of them I wouldn't even know how to
test them. Even just runing orangefs was a lot of additional time and work. So
we'll need to watch out for any issues.

/* Conflicts */
Please note, that linux-next reported a build failure with the NTFS3 tree. The
NTFS3 tree contains changes to their POSIX ACL handling that rely on a helper
that got renamed in this series. Stephen reported and posted an accurate simple
patch to fix this. He suggested to simply point you to the reported failure and
the patch:

https://lore.kernel.org/lkml/20221115101756.5d311f25@canb.auug.org.au

There were no merge conflicts when doing a test merge with current mainline
based on the v6.1 pushed yesterday.
mainline.

The following changes since commit 9abf2313adc1ca1b6180c508c25f22f9395cc780:

  Linux 6.1-rc1 (2022-10-16 15:36:24 -0700)

are available in the Git repository at:

  ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.rework.v6.2

for you to fetch changes up to d6fdf29f7b99814d3673f2d9f4649262807cb836:

  posix_acl: Fix the type of sentinel in get_acl (2022-12-02 10:01:28 +0100)

Please consider pulling these changes from the signed fs.acl.rework.v6.2 tag.

Thanks!
Christian

----------------------------------------------------------------
fs.acl.rework.v6.2

----------------------------------------------------------------
Christian Brauner (35):
      orangefs: rework posix acl handling when creating new filesystem objects
      fs: pass dentry to set acl method
      fs: rename current get acl method
      fs: add new get acl method
      cifs: implement get acl method
      cifs: implement set acl method
      9p: implement get acl method
      9p: implement set acl method
      security: add get, remove and set acl hook
      selinux: implement get, set and remove acl hook
      smack: implement get, set and remove acl hook
      integrity: implement get and set acl hook
      evm: add post set acl hook
      internal: add may_write_xattr()
      acl: add vfs_set_acl()
      acl: add vfs_get_acl()
      acl: add vfs_remove_acl()
      ksmbd: use vfs_remove_acl()
      ecryptfs: implement get acl method
      ecryptfs: implement set acl method
      ovl: implement get acl method
      ovl: implement set acl method
      ovl: use posix acl api
      xattr: use posix acl api
      evm: remove evm_xattr_acl_change()
      ecryptfs: use stub posix acl handlers
      ovl: use stub posix acl handlers
      cifs: use stub posix acl handlers
      9p: use stub posix acl handlers
      acl: remove a slew of now unused helpers
      acl: make vfs_posix_acl_to_xattr() static
      cifs: check whether acl is valid early
      evm: remove dead code in evm_inode_set_acl()
      ovl: call posix_acl_release() after error checking
      orangefs: fix mode handling

Uros Bizjak (1):
      posix_acl: Fix the type of sentinel in get_acl

 Documentation/filesystems/locking.rst |  10 +-
 Documentation/filesystems/porting.rst |   4 +-
 Documentation/filesystems/vfs.rst     |   5 +-
 fs/9p/acl.c                           | 295 +++++++-------
 fs/9p/acl.h                           |   8 +-
 fs/9p/vfs_inode_dotl.c                |   4 +
 fs/9p/xattr.c                         |   7 +-
 fs/9p/xattr.h                         |   2 -
 fs/bad_inode.c                        |   4 +-
 fs/btrfs/acl.c                        |   3 +-
 fs/btrfs/ctree.h                      |   2 +-
 fs/btrfs/inode.c                      |   8 +-
 fs/ceph/acl.c                         |   3 +-
 fs/ceph/dir.c                         |   2 +-
 fs/ceph/inode.c                       |   4 +-
 fs/ceph/super.h                       |   2 +-
 fs/cifs/cifsacl.c                     | 139 +++++++
 fs/cifs/cifsfs.c                      |   4 +
 fs/cifs/cifsproto.h                   |  20 +-
 fs/cifs/cifssmb.c                     | 206 ++++++----
 fs/cifs/xattr.c                       |  68 +---
 fs/ecryptfs/inode.c                   |  32 ++
 fs/erofs/inode.c                      |   6 +-
 fs/erofs/namei.c                      |   2 +-
 fs/ext2/acl.c                         |   3 +-
 fs/ext2/acl.h                         |   2 +-
 fs/ext2/file.c                        |   2 +-
 fs/ext2/inode.c                       |   2 +-
 fs/ext2/namei.c                       |   4 +-
 fs/ext4/acl.c                         |   3 +-
 fs/ext4/acl.h                         |   2 +-
 fs/ext4/file.c                        |   2 +-
 fs/ext4/ialloc.c                      |   2 +-
 fs/ext4/inode.c                       |   2 +-
 fs/ext4/namei.c                       |   4 +-
 fs/f2fs/acl.c                         |   4 +-
 fs/f2fs/acl.h                         |   2 +-
 fs/f2fs/file.c                        |   4 +-
 fs/f2fs/namei.c                       |   4 +-
 fs/fuse/acl.c                         |   3 +-
 fs/fuse/dir.c                         |   4 +-
 fs/fuse/fuse_i.h                      |   2 +-
 fs/gfs2/acl.c                         |   3 +-
 fs/gfs2/acl.h                         |   2 +-
 fs/gfs2/inode.c                       |   6 +-
 fs/internal.h                         |  21 +
 fs/jffs2/acl.c                        |   3 +-
 fs/jffs2/acl.h                        |   2 +-
 fs/jffs2/dir.c                        |   2 +-
 fs/jffs2/file.c                       |   2 +-
 fs/jffs2/fs.c                         |   2 +-
 fs/jfs/acl.c                          |   3 +-
 fs/jfs/file.c                         |   4 +-
 fs/jfs/jfs_acl.h                      |   2 +-
 fs/jfs/namei.c                        |   2 +-
 fs/ksmbd/smb2pdu.c                    |   8 +-
 fs/ksmbd/smbacl.c                     |   6 +-
 fs/ksmbd/vfs.c                        |  21 +-
 fs/ksmbd/vfs.h                        |   4 +-
 fs/namei.c                            |   4 +-
 fs/nfs/nfs3_fs.h                      |   2 +-
 fs/nfs/nfs3acl.c                      |   9 +-
 fs/nfs/nfs3proc.c                     |   4 +-
 fs/nfsd/nfs2acl.c                     |   8 +-
 fs/nfsd/nfs3acl.c                     |   8 +-
 fs/nfsd/nfs4acl.c                     |   4 +-
 fs/nfsd/vfs.c                         |   4 +-
 fs/ntfs3/file.c                       |   4 +-
 fs/ntfs3/namei.c                      |   4 +-
 fs/ntfs3/ntfs_fs.h                    |   4 +-
 fs/ntfs3/xattr.c                      |   9 +-
 fs/ocfs2/acl.c                        |   3 +-
 fs/ocfs2/acl.h                        |   2 +-
 fs/ocfs2/file.c                       |   4 +-
 fs/ocfs2/namei.c                      |   2 +-
 fs/orangefs/acl.c                     |  47 +--
 fs/orangefs/inode.c                   |  54 ++-
 fs/orangefs/namei.c                   |   2 +-
 fs/orangefs/orangefs-kernel.h         |   7 +-
 fs/overlayfs/copy_up.c                |  38 ++
 fs/overlayfs/dir.c                    |  22 +-
 fs/overlayfs/inode.c                  | 187 +++++++--
 fs/overlayfs/overlayfs.h              |  42 +-
 fs/overlayfs/super.c                  | 107 +----
 fs/posix_acl.c                        | 725 +++++++++++++++++-----------------
 fs/reiserfs/acl.h                     |   6 +-
 fs/reiserfs/file.c                    |   2 +-
 fs/reiserfs/inode.c                   |   2 +-
 fs/reiserfs/namei.c                   |   4 +-
 fs/reiserfs/xattr_acl.c               |  11 +-
 fs/xattr.c                            |  85 ++--
 fs/xfs/xfs_acl.c                      |   3 +-
 fs/xfs/xfs_acl.h                      |   2 +-
 fs/xfs/xfs_iops.c                     |  16 +-
 include/linux/evm.h                   |  49 +++
 include/linux/fs.h                    |  10 +-
 include/linux/ima.h                   |  24 ++
 include/linux/lsm_hook_defs.h         |   6 +
 include/linux/lsm_hooks.h             |  12 +
 include/linux/posix_acl.h             |  41 +-
 include/linux/posix_acl_xattr.h       |  47 ++-
 include/linux/security.h              |  29 ++
 include/linux/xattr.h                 |   6 +
 mm/shmem.c                            |   2 +-
 security/integrity/evm/evm_main.c     | 146 ++++---
 security/integrity/ima/ima_appraise.c |   9 +
 security/security.c                   |  42 ++
 security/selinux/hooks.c              |  22 ++
 security/smack/smack_lsm.c            |  71 ++++
 109 files changed, 1820 insertions(+), 1117 deletions(-)

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-12 11:19 [GIT PULL] acl updates for v6.2 Christian Brauner
@ 2022-12-13  2:56 ` Linus Torvalds
  2022-12-13  9:09   ` Christian Brauner
  2022-12-13  3:49 ` pr-tracker-bot
  2022-12-27 15:27 ` J. R. Okajima
  2 siblings, 1 reply; 15+ messages in thread
From: Linus Torvalds @ 2022-12-13  2:56 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel, linux-kernel

On Mon, Dec 12, 2022 at 3:19 AM Christian Brauner <brauner@kernel.org> wrote:
>
>    For a long and detailed
> explanation for just some of the issues [1] provides a good summary.

There is no link [1].

> A few implementation details:
>
> * The series makes sure to retain exactly the same security and integrity module
>   permission checks. See [2] for annotated callchains.

There is no link [2].

This was an extensive changelog for my merge commit, so it's all fine
and I've pulled it, but it does look like some pieces were either
missing, or there was a bit of a cut-and-paste from previous
explanations without the links..

              Linus

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-12 11:19 [GIT PULL] acl updates for v6.2 Christian Brauner
  2022-12-13  2:56 ` Linus Torvalds
@ 2022-12-13  3:49 ` pr-tracker-bot
  2022-12-27 15:27 ` J. R. Okajima
  2 siblings, 0 replies; 15+ messages in thread
From: pr-tracker-bot @ 2022-12-13  3:49 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Linus Torvalds, Christian Brauner, linux-fsdevel, linux-kernel

The pull request you sent on Mon, 12 Dec 2022 12:19:19 +0100:

> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.rework.v6.2

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/6a518afcc2066732e6c5c24281ce017bbbd85506

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-13  2:56 ` Linus Torvalds
@ 2022-12-13  9:09   ` Christian Brauner
  0 siblings, 0 replies; 15+ messages in thread
From: Christian Brauner @ 2022-12-13  9:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fsdevel, linux-kernel

On Mon, Dec 12, 2022 at 06:56:59PM -0800, Linus Torvalds wrote:
> On Mon, Dec 12, 2022 at 3:19 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> >    For a long and detailed
> > explanation for just some of the issues [1] provides a good summary.
> 
> There is no link [1].
> 
> > A few implementation details:
> >
> > * The series makes sure to retain exactly the same security and integrity module
> >   permission checks. See [2] for annotated callchains.
> 
> There is no link [2].
> 
> This was an extensive changelog for my merge commit, so it's all fine
> and I've pulled it, but it does look like some pieces were either
> missing, or there was a bit of a cut-and-paste from previous
> explanations without the links..

Bah, there was a single stray word "mainline" in the pr after the
/* Conflicts */ section because I copied it over a small (relatively
uninteresting) paragraph.

So the two missing links are:

[1]: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org
[2]: https://gist.github.com/brauner/12c795b93a05dc3b3056b1982549a633

they are also listed in the cover letter for the series.

I think I might need a script to look for missing links in the pull
request. I've had a missing link before. Hopefully I'll get around to
this.

About the cut-and-paste: What follows is just my personal
theory/preference but I think it helps to understand how the pr message
come together. If the series is a self-contained topic and not a
collection of a pile of commits than I aim for the description given in
the cover letter and the description given in the merge/pull request
message to be almost indistinguishable. So the cover letter I keep in
git edit --branch-description will morph into the merge/pull request
message.

So I often will try to write the cover letter in a form that is suitable
for the merge commit. Of course, the cover letter will often contain
additional technical information that might not be suitable for the
cover letter. Such sections will then be cut our or rewritten for the pr
message.

Christian

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-12 11:19 [GIT PULL] acl updates for v6.2 Christian Brauner
  2022-12-13  2:56 ` Linus Torvalds
  2022-12-13  3:49 ` pr-tracker-bot
@ 2022-12-27 15:27 ` J. R. Okajima
  2022-12-27 18:31   ` Christian Brauner
  2022-12-28  8:40   ` [GIT PULL] acl updates for v6.2 #forregzbot Thorsten Leemhuis
  2 siblings, 2 replies; 15+ messages in thread
From: J. R. Okajima @ 2022-12-27 15:27 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel

Hello,

Christian Brauner:
> This series passes the LTP and xfstests suites without any regressions. For
> xfstests the following combinations were tested:

I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).

----------------------------------------
on v6.1
+ ls -ld /dev/shm/rw/hd-test/newdir
drwxrwsr-x 2 nobody nogroup 1024 Dec 27 14:46 /dev/shm/rw/hd-test/newdir

+ getfacl -d /dev/shm/rw/hd-test/newdir
# file: dev/shm/rw/hd-test/newdir
# owner: nobody
# group: nogroup
# flags: -s-

----------------------------------------
on v6.2-rc1
+ ls -ld /dev/shm/rw/hd-test/newdir
drwxrwsr-x+ 2 nobody nogroup 1024 Dec 27 23:51 /dev/shm/rw/hd-test/newdir

+ getfacl -d /dev/shm/rw/hd-test/newdir
# file: dev/shm/rw/hd-test/newdir
# owner: nobody
# group: nogroup
# flags: -s-
user::rwx
user:root:rwx
group::r-x
mask::rwx
other::r-x

----------------------------------------

- in the output from 'ls -l', the extra '+' appears
- in the output from 'getfacl -d', some lines are appended
- in those lines, I am not sure whether 'user:root:rwx' is correct or
  not. Even it is correct, getfacl on v6.1 didn't produce such lines.

Is this change intentional?
In other words, is this patch series for a bugfix?


J. R. Okajima

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-27 15:27 ` J. R. Okajima
@ 2022-12-27 18:31   ` Christian Brauner
  2022-12-27 23:54     ` hooanon05g
  2023-01-04  0:57     ` hooanon05g
  2022-12-28  8:40   ` [GIT PULL] acl updates for v6.2 #forregzbot Thorsten Leemhuis
  1 sibling, 2 replies; 15+ messages in thread
From: Christian Brauner @ 2022-12-27 18:31 UTC (permalink / raw)
  To: J. R. Okajima; +Cc: linux-fsdevel

On Wed, Dec 28, 2022 at 12:27:55AM +0900, J. R. Okajima wrote:
> Hello,
> 
> Christian Brauner:
> > This series passes the LTP and xfstests suites without any regressions. For
> > xfstests the following combinations were tested:
> 
> I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).

Hey, I'll try to take a look before new years.

But what xfstests exactly is reporting a failure?
What xfstests config did you use?
How can I reproduce this?
Did you bisect it to this series specifically?

Thanks!
Christian

> 
> ----------------------------------------
> on v6.1
> + ls -ld /dev/shm/rw/hd-test/newdir
> drwxrwsr-x 2 nobody nogroup 1024 Dec 27 14:46 /dev/shm/rw/hd-test/newdir
> 
> + getfacl -d /dev/shm/rw/hd-test/newdir
> # file: dev/shm/rw/hd-test/newdir
> # owner: nobody
> # group: nogroup
> # flags: -s-
> 
> ----------------------------------------
> on v6.2-rc1
> + ls -ld /dev/shm/rw/hd-test/newdir
> drwxrwsr-x+ 2 nobody nogroup 1024 Dec 27 23:51 /dev/shm/rw/hd-test/newdir
> 
> + getfacl -d /dev/shm/rw/hd-test/newdir
> # file: dev/shm/rw/hd-test/newdir
> # owner: nobody
> # group: nogroup
> # flags: -s-
> user::rwx
> user:root:rwx
> group::r-x
> mask::rwx
> other::r-x
> 
> ----------------------------------------
> 
> - in the output from 'ls -l', the extra '+' appears
> - in the output from 'getfacl -d', some lines are appended
> - in those lines, I am not sure whether 'user:root:rwx' is correct or
>   not. Even it is correct, getfacl on v6.1 didn't produce such lines.
> 
> Is this change intentional?
> In other words, is this patch series for a bugfix?
> 
> 
> J. R. Okajima

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-27 18:31   ` Christian Brauner
@ 2022-12-27 23:54     ` hooanon05g
  2023-01-04  0:57     ` hooanon05g
  1 sibling, 0 replies; 15+ messages in thread
From: hooanon05g @ 2022-12-27 23:54 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel

Christian Brauner:
> Hey, I'll try to take a look before new years.
>
> But what xfstests exactly is reporting a failure?
> What xfstests config did you use?
> How can I reproduce this?
> Did you bisect it to this series specifically?

It is not xfstests.
It is just a part of my local tests for another security issue.
Now I am trying creating a simplest reproducible senario, but it may
take some time. May be a few weeks.
So I'd say "Have nice holidays, and thanx for the reply."


J. R. Okajima

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

* Re: [GIT PULL] acl updates for v6.2 #forregzbot
  2022-12-27 15:27 ` J. R. Okajima
  2022-12-27 18:31   ` Christian Brauner
@ 2022-12-28  8:40   ` Thorsten Leemhuis
  2023-01-01 20:07     ` J. R. Okajima
  1 sibling, 1 reply; 15+ messages in thread
From: Thorsten Leemhuis @ 2022-12-28  8:40 UTC (permalink / raw)
  To: regressions; +Cc: linux-fsdevel

[Note: this mail contains only information for Linux kernel regression
tracking. Mails like these contain '#forregzbot' in the subject to make
then easy to spot and filter out. The author also tried to remove most
or all individuals from the list of recipients to spare them the hassle.]

On 27.12.22 16:27, J. R. Okajima wrote:
> 
> Christian Brauner:
>> This series passes the LTP and xfstests suites without any regressions. For
>> xfstests the following combinations were tested:
> 
> I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced v6.1..v6.2-rc1
#regzbot title fs: ext3/acl: behavior changed (ls and getact show
slightly different output)
#regzbot ignore-activity

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

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

> ----------------------------------------
> on v6.1
> + ls -ld /dev/shm/rw/hd-test/newdir
> drwxrwsr-x 2 nobody nogroup 1024 Dec 27 14:46 /dev/shm/rw/hd-test/newdir
> 
> + getfacl -d /dev/shm/rw/hd-test/newdir
> # file: dev/shm/rw/hd-test/newdir
> # owner: nobody
> # group: nogroup
> # flags: -s-
> 
> ----------------------------------------
> on v6.2-rc1
> + ls -ld /dev/shm/rw/hd-test/newdir
> drwxrwsr-x+ 2 nobody nogroup 1024 Dec 27 23:51 /dev/shm/rw/hd-test/newdir
> 
> + getfacl -d /dev/shm/rw/hd-test/newdir
> # file: dev/shm/rw/hd-test/newdir
> # owner: nobody
> # group: nogroup
> # flags: -s-
> user::rwx
> user:root:rwx
> group::r-x
> mask::rwx
> other::r-x
> 
> ----------------------------------------
> 
> - in the output from 'ls -l', the extra '+' appears
> - in the output from 'getfacl -d', some lines are appended
> - in those lines, I am not sure whether 'user:root:rwx' is correct or
>   not. Even it is correct, getfacl on v6.1 didn't produce such lines.
> 
> Is this change intentional?
> In other words, is this patch series for a bugfix?
> 
> 
> J. R. Okajima

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

* Re: [GIT PULL] acl updates for v6.2 #forregzbot
  2022-12-28  8:40   ` [GIT PULL] acl updates for v6.2 #forregzbot Thorsten Leemhuis
@ 2023-01-01 20:07     ` J. R. Okajima
  2023-01-02  7:25       ` Thorsten Leemhuis
  0 siblings, 1 reply; 15+ messages in thread
From: J. R. Okajima @ 2023-01-01 20:07 UTC (permalink / raw)
  To: Thorsten Leemhuis; +Cc: regressions, linux-fsdevel

Thorsten Leemhuis:
> Thanks for the report. To be sure below issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
> tracking bot:

Hold it!
I'm not sure whether this is a regression or a bugfix. Or even it maybe
a problem on my side. I'm still struggling to find out the reproducible
way.


J. R. Okajima

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

* Re: [GIT PULL] acl updates for v6.2 #forregzbot
  2023-01-01 20:07     ` J. R. Okajima
@ 2023-01-02  7:25       ` Thorsten Leemhuis
  0 siblings, 0 replies; 15+ messages in thread
From: Thorsten Leemhuis @ 2023-01-02  7:25 UTC (permalink / raw)
  To: J. R. Okajima; +Cc: regressions, linux-fsdevel



On 01.01.23 21:07, J. R. Okajima wrote:
> Thorsten Leemhuis:
>> Thanks for the report. To be sure below issue doesn't fall through the
>> cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
>> tracking bot:
> 
> Hold it!
> I'm not sure whether this is a regression or a bugfix. Or even it maybe
> a problem on my side. I'm still struggling to find out the reproducible
> way.

Don't worry about that, it's way easier to keep track of a potential
regression by just adding it to the tracking and later removing it, if
it turns out to not be a regression.

BTW, xfstests found a acl issue on ext4, too:
https://lore.kernel.org/lkml/202212291509.704a11c9-oliver.sang@intel.com/

I have no idea at all if this might be related, as this is not my area
of expertise, I just thought you might want to know about that.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr

Did I miss something or do something stupid? Then reply and tell me:
https://linux-regtracking.leemhuis.info/about/#stupid

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

* Re: [GIT PULL] acl updates for v6.2
  2022-12-27 18:31   ` Christian Brauner
  2022-12-27 23:54     ` hooanon05g
@ 2023-01-04  0:57     ` hooanon05g
  2023-01-04 10:04       ` Linux kernel regression tracking (#info)
  1 sibling, 1 reply; 15+ messages in thread
From: hooanon05g @ 2023-01-04  0:57 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel, Thorsten Leemhuis, regressions

Christian Brauner:
> On Wed, Dec 28, 2022 at 12:27:55AM +0900, J. R. Okajima wrote:
	:::
> > I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).
>
> Hey, I'll try to take a look before new years.

Now it becomes clear that the problem was on my side.
The "acl updates for v6.2" in mainline has nothing to deal with it.
Sorry for the noise.


J. R. Okajima

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

* Re: [GIT PULL] acl updates for v6.2
  2023-01-04  0:57     ` hooanon05g
@ 2023-01-04 10:04       ` Linux kernel regression tracking (#info)
  2023-01-04 10:14         ` Christian Brauner
  0 siblings, 1 reply; 15+ messages in thread
From: Linux kernel regression tracking (#info) @ 2023-01-04 10:04 UTC (permalink / raw)
  To: hooanon05g, Christian Brauner; +Cc: linux-fsdevel, regressions

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 04.01.23 01:57, hooanon05g@gmail.com wrote:
> Christian Brauner:
>> On Wed, Dec 28, 2022 at 12:27:55AM +0900, J. R. Okajima wrote:
> 	:::
>>> I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).
>>
>> Hey, I'll try to take a look before new years.
> 
> Now it becomes clear that the problem was on my side.
> The "acl updates for v6.2" in mainline has nothing to deal with it.
> Sorry for the noise.

In that case:

#regzbot resolve: turns out it was a local problem and not regression in
the kernel

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
-- 
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

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

* Re: [GIT PULL] acl updates for v6.2
  2023-01-04 10:04       ` Linux kernel regression tracking (#info)
@ 2023-01-04 10:14         ` Christian Brauner
  2023-01-04 10:26           ` Thorsten Leemhuis
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2023-01-04 10:14 UTC (permalink / raw)
  To: Thorsten Leemhuis; +Cc: hooanon05g, linux-fsdevel, regressions

On Wed, Jan 04, 2023 at 11:04:06AM +0100, Linux kernel regression tracking (#info) wrote:
> [TLDR: This mail in primarily relevant for Linux kernel regression
> tracking. See link in footer if these mails annoy you.]
> 
> On 04.01.23 01:57, hooanon05g@gmail.com wrote:
> > Christian Brauner:
> >> On Wed, Dec 28, 2022 at 12:27:55AM +0900, J. R. Okajima wrote:
> > 	:::
> >>> I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).
> >>
> >> Hey, I'll try to take a look before new years.
> > 
> > Now it becomes clear that the problem was on my side.
> > The "acl updates for v6.2" in mainline has nothing to deal with it.
> > Sorry for the noise.
> 
> In that case:
> 
> #regzbot resolve: turns out it was a local problem and not regression in
> the kernel

When and how did regzbot start tracking this? None of the mails that
reported this issue to me contained any reference to regzbot.

If something is currently classified as a regression it'd be good to let
the responsible maintainers and developers know that.

Christian

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

* Re: [GIT PULL] acl updates for v6.2
  2023-01-04 10:14         ` Christian Brauner
@ 2023-01-04 10:26           ` Thorsten Leemhuis
  2023-01-04 10:37             ` Christian Brauner
  0 siblings, 1 reply; 15+ messages in thread
From: Thorsten Leemhuis @ 2023-01-04 10:26 UTC (permalink / raw)
  To: Christian Brauner; +Cc: hooanon05g, linux-fsdevel, regressions



On 04.01.23 11:14, Christian Brauner wrote:
> On Wed, Jan 04, 2023 at 11:04:06AM +0100, Linux kernel regression tracking (#info) wrote:
>> [TLDR: This mail in primarily relevant for Linux kernel regression
>> tracking. See link in footer if these mails annoy you.]
>>
>> On 04.01.23 01:57, hooanon05g@gmail.com wrote:
>>> Christian Brauner:
>>>> On Wed, Dec 28, 2022 at 12:27:55AM +0900, J. R. Okajima wrote:
>>> 	:::
>>>>> I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).
>>>>
>>>> Hey, I'll try to take a look before new years.
>>>
>>> Now it becomes clear that the problem was on my side.
>>> The "acl updates for v6.2" in mainline has nothing to deal with it.
>>> Sorry for the noise.
>>
>> In that case:
>>
>> #regzbot resolve: turns out it was a local problem and not regression in
>> the kernel
> 
> When and how did regzbot start tracking this? None of the mails that
> reported this issue to me contained any reference to regzbot.
> 
> If something is currently classified as a regression it'd be good to let
> the responsible maintainers and developers know that.

That happens these days (again) -- since five, too be more precise, as I
recently (again) changed how to handle that mails (some maintainers
hated getting mails about adding reports to the tracking, but I recently
 decided they have to deal with that locally).

But the mail that added this thread to the tracking was before that,
sorry. :-/
https://lore.kernel.org/all/2aa5cc7e-ca00-22a7-5e2f-7eb73556181e@leemhuis.info/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

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

* Re: [GIT PULL] acl updates for v6.2
  2023-01-04 10:26           ` Thorsten Leemhuis
@ 2023-01-04 10:37             ` Christian Brauner
  0 siblings, 0 replies; 15+ messages in thread
From: Christian Brauner @ 2023-01-04 10:37 UTC (permalink / raw)
  To: Thorsten Leemhuis; +Cc: hooanon05g, linux-fsdevel, regressions

On Wed, Jan 04, 2023 at 11:26:41AM +0100, Thorsten Leemhuis wrote:
> 
> 
> On 04.01.23 11:14, Christian Brauner wrote:
> > On Wed, Jan 04, 2023 at 11:04:06AM +0100, Linux kernel regression tracking (#info) wrote:
> >> [TLDR: This mail in primarily relevant for Linux kernel regression
> >> tracking. See link in footer if these mails annoy you.]
> >>
> >> On 04.01.23 01:57, hooanon05g@gmail.com wrote:
> >>> Christian Brauner:
> >>>> On Wed, Dec 28, 2022 at 12:27:55AM +0900, J. R. Okajima wrote:
> >>> 	:::
> >>>>> I've found a behaviour got changed from v6.1 to v6.2-rc1 on ext3 (ext4).
> >>>>
> >>>> Hey, I'll try to take a look before new years.
> >>>
> >>> Now it becomes clear that the problem was on my side.
> >>> The "acl updates for v6.2" in mainline has nothing to deal with it.
> >>> Sorry for the noise.
> >>
> >> In that case:
> >>
> >> #regzbot resolve: turns out it was a local problem and not regression in
> >> the kernel
> > 
> > When and how did regzbot start tracking this? None of the mails that
> > reported this issue to me contained any reference to regzbot.
> > 
> > If something is currently classified as a regression it'd be good to let
> > the responsible maintainers and developers know that.
> 
> That happens these days (again) -- since five, too be more precise, as I
> recently (again) changed how to handle that mails (some maintainers
> hated getting mails about adding reports to the tracking, but I recently
>  decided they have to deal with that locally).
> 
> But the mail that added this thread to the tracking was before that,
> sorry. :-/
> https://lore.kernel.org/all/2aa5cc7e-ca00-22a7-5e2f-7eb73556181e@leemhuis.info/

Ok, no problem. It's just odd because the mail you link here is a reply to
https://lore.kernel.org/all/29161.1672154875@jrobl
which has both mine and the original reporters mail address. Which means
a reply to it could've just kept all recipients which means that they
somehow must've been removed before it got sent.

Just in the future it would be great to be notified early when something
is classified as a regression.

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

end of thread, other threads:[~2023-01-04 10:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-12 11:19 [GIT PULL] acl updates for v6.2 Christian Brauner
2022-12-13  2:56 ` Linus Torvalds
2022-12-13  9:09   ` Christian Brauner
2022-12-13  3:49 ` pr-tracker-bot
2022-12-27 15:27 ` J. R. Okajima
2022-12-27 18:31   ` Christian Brauner
2022-12-27 23:54     ` hooanon05g
2023-01-04  0:57     ` hooanon05g
2023-01-04 10:04       ` Linux kernel regression tracking (#info)
2023-01-04 10:14         ` Christian Brauner
2023-01-04 10:26           ` Thorsten Leemhuis
2023-01-04 10:37             ` Christian Brauner
2022-12-28  8:40   ` [GIT PULL] acl updates for v6.2 #forregzbot Thorsten Leemhuis
2023-01-01 20:07     ` J. R. Okajima
2023-01-02  7:25       ` Thorsten Leemhuis

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.