[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:
Related
is related to LU-9679 Prepare lustre for adoption into the ... Resolved
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
Subject: LU-10785 llite: use xattr_handler name for ACLs
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 269568dea51bcd678183402f3898b484fce08627

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
sanity: FAIL: test_102b can't get trusted.lov from /lustre/lustre/f102b.sanity
sanity: FAIL: test_103a run_acl_subtest cp failed

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/
Subject: LU-10785 llite: use xattr_handler name for ACLs
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: a9998927281647f9d9b8bfa50c80b7e15cf05eda

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

Generated at Sat Feb 10 02:38:10 UTC 2024 using Jira 9.4.14#940014-sha1:734e6822bbf0d45eff9af51f82432957f73aa32c.