Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'applyto' fails on Rocky 8 with "Invalid argument" since an ACL with named user/group doesn't have a mask ACE #26

Open
liorsvast opened this issue Jan 7, 2025 · 9 comments

Comments

@liorsvast
Copy link

Issue

According to posix_acl_valid in the Linux kernel, a mask entry is needed if there are any user or group entries.

posix_acl_valid reference

https://github.com/kernelim/linux/blob/linux-4.18.0-553.30.1.el8_10.tar.xz/fs/posix_acl.c#L209

Reproduction

$ uname -r
4.18.0-553.30.1.el8_10.x86_64
$ python3.11
Python 3.11.5 (main, Nov 15 2023, 18:13:17) [GCC 8.5.0 20210514 (Red Hat 8.5.0-20.0.1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import posix1e
>>> text_access_acl = "u::r,g::r,o::r,u:456:r"
>>> access_acl = posix1e.ACL(text=text_access_acl)
>>> access_acl.applyto("/mnt/nfs3_mount/dir1", posix1e.ACL_TYPE_ACCESS)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 22] Invalid argument: '/mnt/nfs3_mount/dir1'
@iustin
Copy link
Owner

iustin commented Jan 7, 2025

I'm not sure what the bug report is about. As you say, if there are user/group entries, a mask is needed, and you didn't pass it.

Either pass an explicit mask manually, or use the calc_mask before applying:

$ python3
Python 3.11.2 (main, Sep 14 2024, 03:00:30) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import posix1e
>>> text_access_acl = "u::r,g::r,o::r,u:456:r"
>>> access_acl = posix1e.ACL(text=text_access_acl)
>>> access_acl.calc_mask()
>>> print(access_acl)
user::r--
user:456:r--
group::r--
mask::r--
other::r--

>>> access_acl.applyto('z')

The reason why the mask is not automatically applied is to keep parity with the C library, and allow customisation of the mask.

@liorsvast
Copy link
Author

Thanks for the quick response.

This may break, e.g., existing apps that transition from CentOS 7 to Rocky 8.

Perhaps we can implicitly set the mask if the ACL doesn't have one and yet it needs one?

@iustin
Copy link
Owner

iustin commented Jan 7, 2025

Hmm, are you sure this is a regression? The code here, and in the Linux kernel, hasn't changed in a long time, I believe.

@liorsvast
Copy link
Author

From what I saw, CentOS 7 doesn't call posix_acl_valid at all.

CentOS 7 - 3.10.0-1160.118.1.el7.x86_64

    setxattr
        vfs_setxattr
            __vfs_setxattr_noperm
                struct inode *inode = dentry->d_inode
                if (inode->i_op->setxattr)
                    inode->i_op->setxattr()
                        (inode->i_op probably points to nfs3_dir_inode_operations)
                            nfs3_setxattr
                                posix_acl_from_xattr

Rocky 8 - 4.18.0-553.30.1.el8_10.x86_64

    setxattr
        vfs_setxattr
            __vfs_setxattr_locked
                __vfs_setxattr_noperm
                    __vfs_setxattr
                        handler = xattr_resolve_name(inode, &name);
                        handler->set()
                            (we probably have handler 'posix_acl_default_xattr_handler')
                                posix_acl_xattr_set
                                    if (value)
                                        posix_acl_from_xattr
                                    set_posix_acl
                                        if (acl)
                                            posix_acl_valid <- We fail here

@iustin
Copy link
Owner

iustin commented Jan 7, 2025

So I haven't ever seen, on XFS at least, applying an ACL with an invalid mask to pass. Just to confirm - can you tell me which filesystem you're using in both cases, and which are the kernel versions?

@liorsvast
Copy link
Author

I'm using NFSv3 filesystem and the kernel versions are mentioned in my previous comment.

  1. 3.10.0-1160.118.1.el7.x86_64 - pass
  2. 4.18.0-553.30.1.el8_10.x86_64 - fail

@iustin
Copy link
Owner

iustin commented Jan 8, 2025

NFS, I'm not surprised then. But those versions are package versions, not actual kernel versions. Can you tell me the output of uname -r?

@liorsvast
Copy link
Author

it's the same. please see it in my original post.

@iustin
Copy link
Owner

iustin commented Jan 8, 2025

Wait, you're actually talking about 3.x and 4.x kernels? From many many years ago?

Please try to confirm behaviour on a modern (6.x) kernel. Honestly, I'm not inclined to handle upgrades from kernel 3.x, since that's ancient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants