Actions
Bug #21004
closedfs: client/mds has wrong check to clear S_ISGID on chown
% Done:
0%
Source:
Q/A
Tags:
Backport:
luminous
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
MDS, ceph-fuse, kceph
Labels (FS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):
Description
Reported in: https://bugzilla.redhat.com/show_bug.cgi?id=1480182
This causes the failure in test 88 from https://bugzilla.redhat.com/attachment.cgi?id=1311693
Updated by Patrick Donnelly over 6 years ago
- Status changed from New to In Progress
Updated by Patrick Donnelly over 6 years ago
- Subject changed from fs: client/mds do not always clear S_ISGID on chown to fs: client/mds has wrong check to clear S_ISGID on chown
Well actually the test above fails because the chown was a no-op due to an earlier chown failure. In any case I've found that S_ISGID is not cleared correctly which I identified in my own testing.
The condition should be:
- } else if (kill_sguid && S_ISREG(in->mode)) { + } else if (kill_sguid && S_ISREG(in->mode) && (in->mode & (S_IXUSR|S_IXGRP|S_IXOTH))) { /* Must squash the any setuid/setgid bits with an ownership change */ - in->mode &= ~S_ISUID; - if ((in->mode & (S_ISGID|S_IXGRP)) == (S_ISGID|S_IXGRP)) - in->mode &= ~S_ISGID; + in->mode &= ~(S_ISUID|S_ISGID);
According to: http://pubs.opengroup.org/onlinepubs/9699919799/
Updated by Zheng Yan over 6 years ago
The logical is from kernel_src/fs/attr.c
if (ia_valid & ATTR_KILL_SUID) { if (mode & S_ISUID) { ia_valid = attr->ia_valid |= ATTR_MODE; attr->ia_mode = (inode->i_mode & ~S_ISUID); } } if (ia_valid & ATTR_KILL_SGID) { if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { if (!(ia_valid & ATTR_MODE)) { ia_valid = attr->ia_valid |= ATTR_MODE; attr->ia_mode = inode->i_mode; } attr->ia_mode &= ~S_ISGID; } }
Besides, it's possible that mds does not handle setattr request for chmod correctly. (client does not have CEPH_CAP_AUTH_EXCL)
Updated by Patrick Donnelly over 6 years ago
Updated by Patrick Donnelly over 6 years ago
- Status changed from In Progress to Fix Under Review
Updated by Patrick Donnelly over 6 years ago
- Status changed from Fix Under Review to Pending Backport
Updated by Nathan Cutler over 6 years ago
- Copied to Backport #21107: luminous: fs: client/mds has wrong check to clear S_ISGID on chown added
Updated by Nathan Cutler over 6 years ago
- Status changed from Pending Backport to Resolved
Actions