Skip to content

Commit b4ada52

Browse files
anodos325Ryan Moeller
authored andcommitted
Fix access check when cred allows override of ACL
Properly evaluate edge cases where user credential may grant capability to override DAC in various situations. Switch to using ns-aware checks rather than capable(). Expand optimization allow bypass of zfs_zaccess() in case of trivial ACL if MAY_OPEN is included in requested mask. This will be evaluated in generic_permission() check, which is RCU walk safe. This means that in most cases evaluating permissions on boot volume with NFSv4 ACLs will follow the fast path on checking inode permissions. Additionally, CAP_SYS_ADMIN is granted to nfsd process, and so override for this capability in access2 policy check is removed in favor of a simple check for fsid == 0. Checks for CAP_DAC_OVERRIDE and other override capabilities are kept as-is. Signed-off-by: Andrew Walker <[email protected]>
1 parent f6a60ab commit b4ada52

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

module/os/linux/zfs/policy.c

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,38 +114,45 @@ secpolicy_vnode_access2(const cred_t *cr, struct inode *ip, uid_t owner,
114114
mode_t curmode, mode_t wantmode)
115115
{
116116
mode_t remainder = ~curmode & wantmode;
117+
uid_t uid = crgetuid(cr);
117118
if ((ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4) ||
118119
(remainder == 0)) {
119120
return (0);
120121
}
121122

122-
/*
123-
* short-circuit if root
124-
*/
125-
if (capable(CAP_SYS_ADMIN)) {
123+
if ((uid == owner) || (uid == 0))
126124
return (0);
127-
}
125+
126+
if (zpl_inode_owner_or_capable(kcred->user_ns, ip))
127+
return (0);
128+
129+
#if defined(CONFIG_USER_NS)
130+
if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner)))
131+
return (EPERM);
132+
#endif
128133

129134
/*
130135
* There are some situations in which capabilities
131136
* may allow overriding the DACL.
132137
*/
133138
if (S_ISDIR(ip->i_mode)) {
134139
if (!(wantmode & S_IWUSR) &&
135-
capable(CAP_DAC_READ_SEARCH)) {
140+
(priv_policy_user(cr, CAP_DAC_READ_SEARCH, EPERM) == 0)) {
136141
return (0);
137142
}
138-
if (capable(CAP_DAC_OVERRIDE)) {
143+
if (priv_policy_user(cr, CAP_DAC_OVERRIDE, EPERM) == 0) {
139144
return (0);
140145
}
141146
return (EACCES);
142147
}
143148

144-
if ((wantmode == S_IRUSR) && capable(CAP_DAC_READ_SEARCH)) {
149+
if ((wantmode == S_IRUSR) &&
150+
(priv_policy_user(cr, CAP_DAC_READ_SEARCH, EPERM) == 0)) {
145151
return (0);
146152
}
147153

148-
if (!(remainder & S_IXUSR) && capable(CAP_DAC_OVERRIDE)) {
154+
if (!(remainder & S_IXUSR) &&
155+
(priv_policy_user(cr, CAP_DAC_OVERRIDE, EPERM) == 0)) {
149156
return (0);
150157
}
151158

module/os/linux/zfs/zpl_xattr.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ static const struct {
106106
#endif
107107
};
108108

109-
#define GENERIC_MASK(mask) ((mask & ~(MAY_READ | MAY_WRITE | MAY_EXEC)) == 0)
109+
#define POSIX_MASKS (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_OPEN)
110+
#define GENERIC_MASK(mask) ((mask & ~POSIX_MASKS) == 0)
110111

111112
enum xattr_permission {
112113
XAPERM_DENY,

0 commit comments

Comments
 (0)