[LU-10785] ll_acl_{access,default}_xattr_handler confuse name and prefix Created: 07/Mar/18 Updated: 09/Apr/18 Resolved: 09/Apr/18 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | Lustre 2.11.0, Lustre 2.10.3 |
| Fix Version/s: | Lustre 2.12.0 |
| Type: | Bug | Priority: | Minor |
| Reporter: | John Hammond | Assignee: | John Hammond |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Issue Links: |
|
||||||||
| Severity: | 3 | ||||||||
| Rank (Obsolete): | 9223372036854775807 | ||||||||
| Description |
|
In the definition of ll_acl_access_xattr_handler we use the xattr name ("system.posix_acl_access") to initialize the prefix member: static const struct xattr_handler ll_acl_access_xattr_handler = { .prefix = XATTR_NAME_POSIX_ACL_ACCESS, .flags = XATTR_ACL_ACCESS_T, #if defined(HAVE_XATTR_HANDLER_SIMPLIFIED) .get = ll_xattr_get_common_4_3, .set = ll_xattr_set_common_4_3, #elif !defined(HAVE_XATTR_HANDLER_INODE_PARAM) .get = ll_xattr_get_common_3_11, .set = ll_xattr_set_common_3_11, #else .get = ll_xattr_get_common, .set = ll_xattr_set_common, #endif }; We should leave prefix unassigned and use the name member instead. This causes getfacl() (which uses getxattr()) to fail for some kernel versions. Similarly for ll_acl_default_xattr_handler. There also may be conflicts between Lustre and upstream regarding the use of the flags member of struct xattr_handler. Lustre uses #define XATTR_ACL_ACCESS_T 4 #define XATTR_ACL_DEFAULT_T 5 Upstream uses: /* a_type field in acl_user_posix_entry_t */
#define ACL_TYPE_ACCESS (0x8000)
#define ACL_TYPE_DEFAULT (0x4000)
I did not try to determine how the kernel uses the flags member. Other filesystems use kernel defined handers for ACLs: static const struct xattr_handler *ext4_xattr_handler_map[] = { [EXT4_XATTR_INDEX_USER] = &ext4_xattr_user_handler, #ifdef CONFIG_EXT4_FS_POSIX_ACL [EXT4_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler, [EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler, #endif [EXT4_XATTR_INDEX_TRUSTED] = &ext4_xattr_trusted_handler, #ifdef CONFIG_EXT4_FS_SECURITY [EXT4_XATTR_INDEX_SECURITY] = &ext4_xattr_security_handler, #endif }; We should see if these can be used by Lustre as well. Some auditing is needed. |
| Comments |
| Comment by Gerrit Updater [ 08/Mar/18 ] |
|
John L. Hammond (john.hammond@intel.com) uploaded a new patch: https://review.whamcloud.com/31595 |
| Comment by James A Simmons [ 09/Mar/18 ] |
|
Thank you John for tracking down these issues. I will port to upstream and report back the results |
| Comment by James A Simmons [ 20/Mar/18 ] |
|
I ported your changes to the upstream kernel and ran sanity test 102 and 103. From the test I see the following test fail: sanity: FAIL: test_102a /lustre/lustre/f102a.sanity missing 3 trusted.name xattrs This is testing with your latest patches. |
| Comment by John Hammond [ 22/Mar/18 ] |
|
Did these tests pass before? Can you post your port? |
| Comment by James A Simmons [ 28/Mar/18 ] |
|
Nope. I did some digging and found the bug upstream which caused 102a and 102b to fail. Now it works out of the box. Only 103a fails for the upstream client. Do you see any failures with acl/run_test permisson ? Your patches look great. Thank you!!! |
| Comment by Gerrit Updater [ 09/Apr/18 ] |
|
Oleg Drokin (oleg.drokin@intel.com) merged in patch https://review.whamcloud.com/31595/ |
| Comment by James A Simmons [ 09/Apr/18 ] |
|
Before we close this I like to test Ubunut 16 sanity test again. |
| Comment by Peter Jones [ 09/Apr/18 ] |
|
Landed for 2.12 |
| Comment by James A Simmons [ 09/Apr/18 ] |
|
Just as a note I pushed https://review.whamcloud.com/#/c/31921 to verify these changes |