[ https://issues.apache.org/jira/browse/HDFS-6556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14040346#comment-14040346 ]
Yi Liu commented on HDFS-6556: ------------------------------ Thanks Uma, nice work. Then for user and trusted namespace xattrs, they are in line with linux. I have two small comments: {code} - FSPermissionChecker pc = getPermissionChecker(); - XAttrPermissionFilter.checkPermissionForApi(pc, xAttr); + final FSPermissionChecker pc = isPermissionEnabled ? getPermissionChecker() + : null; + if (pc != null) { + XAttrPermissionFilter.checkPermissionForApi(pc, xAttr); + } {code} I think we don't need this modification. {{getPermissionChecker()}} will always create a {{FSPermissionChecker}}, even without permission enabled, it can be used to check the super. Then it's in line with other places in {{FSNameSystem}} where {{FSPermissionChecker pc = getPermissionChecker();}} is used. {code} - if (xAttr.getNameSpace() == XAttr.NameSpace.USER || - (xAttr.getNameSpace() == XAttr.NameSpace.TRUSTED && - pc.isSuperUser())) { + if (xAttr.getNameSpace() == XAttr.NameSpace.USER) { return; } + + if(xAttr.getNameSpace() == XAttr.NameSpace.TRUSTED){ + pc.checkSuperuserPrivilege(); + return; + } {code} This may be not necessary, indeed {{pc.checkSuperuserPrivilege()}} will check the super and throw exception, but the exception is not clear, I'd prefer original one. > Refine XAttr permissions > ------------------------ > > Key: HDFS-6556 > URL: https://issues.apache.org/jira/browse/HDFS-6556 > Project: Hadoop HDFS > Issue Type: Bug > Components: namenode > Affects Versions: 2.5.0 > Reporter: Yi Liu > Assignee: Uma Maheswara Rao G > Attachments: RefinedPermissions-HDFS-6556-1.patch, > RefinedPermissions-HDFS-6556.patch > > > After discuss with Uma, we should refine setting permissions of {{user}} and > {{trusted}} namespace xattrs. > *1.* For {{user}} namespace xattrs, In HDFS-6374, says "setXAttr should > require the user to be the owner of the file or directory", we have a bit > misunderstanding. It actually is: > {quote} > The access permissions for user attributes are defined by the file permission > bits. only regular files and directories can have extended attributes. For > sticky directories, only the owner and privileged user can write attributes. > {quote} > We can refer to linux source code in > http://lxr.free-electrons.com/source/fs/xattr.c?v=2.6.35 > I also check in linux, it's controlled by the file permission bits for > regular files and directories (not sticky). > *2.* For {{trusted}} namespace, currently we require the user should be owner > and superuser. Actually superuser is enough. -- This message was sent by Atlassian JIRA (v6.2#6252)