Skip to content

Commit

Permalink
[XFS] use inode_change_ok for setattr permission checking
Browse files Browse the repository at this point in the history 
Instead of implementing our own checks use inode_change_ok to check for
necessary permission in setattr.  There is a slight change in behaviour
as inode_change_ok doesn't allow i_mode updates to add the suid or sgid
without superuser privilegues while the old XFS code just stripped away
those bits from the file mode.

(First sent on Semptember 29th)

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>
  • Loading branch information
Christoph Hellwig authored and Lachlan McIlroy committed Dec 11, 2008
1 parent 4d4be48 commit c4cd747
Showing 1 changed file with 36 additions and 113 deletions.
149 changes: 36 additions & 113 deletions fs/xfs/xfs_vnodeops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ xfs_setattr(
gid_t gid=0, igid=0;
int timeflags = 0;
struct xfs_dquot *udqp, *gdqp, *olddquot1, *olddquot2;
int file_owner;
int need_iolock = 1;

xfs_itrace_entry(ip);
Expand All @@ -81,6 +80,10 @@ xfs_setattr(
if (XFS_FORCED_SHUTDOWN(mp))
return XFS_ERROR(EIO);

code = -inode_change_ok(inode, iattr);
if (code)
return code;

olddquot1 = olddquot2 = NULL;
udqp = gdqp = NULL;

Expand Down Expand Up @@ -158,56 +161,6 @@ xfs_setattr(

xfs_ilock(ip, lock_flags);

/* boolean: are we the file owner? */
file_owner = (current_fsuid() == ip->i_d.di_uid);

/*
* Change various properties of a file.
* Only the owner or users with CAP_FOWNER
* capability may do these things.
*/
if (mask & (ATTR_MODE|ATTR_UID|ATTR_GID)) {
/*
* CAP_FOWNER overrides the following restrictions:
*
* The user ID of the calling process must be equal
* to the file owner ID, except in cases where the
* CAP_FSETID capability is applicable.
*/
if (!file_owner && !capable(CAP_FOWNER)) {
code = XFS_ERROR(EPERM);
goto error_return;
}

/*
* CAP_FSETID overrides the following restrictions:
*
* The effective user ID of the calling process shall match
* the file owner when setting the set-user-ID and
* set-group-ID bits on that file.
*
* The effective group ID or one of the supplementary group
* IDs of the calling process shall match the group owner of
* the file when setting the set-group-ID bit on that file
*/
if (mask & ATTR_MODE) {
mode_t m = 0;

if ((iattr->ia_mode & S_ISUID) && !file_owner)
m |= S_ISUID;
if ((iattr->ia_mode & S_ISGID) &&
!in_group_p((gid_t)ip->i_d.di_gid))
m |= S_ISGID;
#if 0
/* Linux allows this, Irix doesn't. */
if ((iattr->ia_mode & S_ISVTX) && !S_ISDIR(ip->i_d.di_mode))
m |= S_ISVTX;
#endif
if (m && !capable(CAP_FSETID))
iattr->ia_mode &= ~m;
}
}

/*
* Change file ownership. Must be the owner or privileged.
*/
Expand All @@ -223,22 +176,6 @@ xfs_setattr(
gid = (mask & ATTR_GID) ? iattr->ia_gid : igid;
uid = (mask & ATTR_UID) ? iattr->ia_uid : iuid;

/*
* CAP_CHOWN overrides the following restrictions:
*
* If _POSIX_CHOWN_RESTRICTED is defined, this capability
* shall override the restriction that a process cannot
* change the user ID of a file it owns and the restriction
* that the group ID supplied to the chown() function
* shall be equal to either the group ID or one of the
* supplementary group IDs of the calling process.
*/
if ((iuid != uid ||
(igid != gid && !in_group_p((gid_t)gid))) &&
!capable(CAP_CHOWN)) {
code = XFS_ERROR(EPERM);
goto error_return;
}
/*
* Do a quota reservation only if uid/gid is actually
* going to change.
Expand Down Expand Up @@ -276,36 +213,22 @@ xfs_setattr(
code = XFS_ERROR(EINVAL);
goto error_return;
}

/*
* Make sure that the dquots are attached to the inode.
*/
if ((code = XFS_QM_DQATTACH(mp, ip, XFS_QMOPT_ILOCKED)))
code = XFS_QM_DQATTACH(mp, ip, XFS_QMOPT_ILOCKED);
if (code)
goto error_return;
}

/*
* Change file access or modified times.
*/
if (mask & (ATTR_ATIME|ATTR_MTIME)) {
if (!file_owner) {
if ((mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)) &&
!capable(CAP_FOWNER)) {
code = XFS_ERROR(EPERM);
goto error_return;
}
}
}

/*
* Now we can make the changes. Before we join the inode
* to the transaction, if ATTR_SIZE is set then take care of
* the part of the truncation that must be done without the
* inode lock. This needs to be done before joining the inode
* to the transaction, because the inode cannot be unlocked
* once it is a part of the transaction.
*/
if (mask & ATTR_SIZE) {
code = 0;
/*
* Now we can make the changes. Before we join the inode
* to the transaction, if ATTR_SIZE is set then take care of
* the part of the truncation that must be done without the
* inode lock. This needs to be done before joining the inode
* to the transaction, because the inode cannot be unlocked
* once it is a part of the transaction.
*/
if (iattr->ia_size > ip->i_size) {
/*
* Do the first part of growing a file: zero any data
Expand Down Expand Up @@ -360,17 +283,10 @@ xfs_setattr(
}
commit_flags = XFS_TRANS_RELEASE_LOG_RES;
xfs_ilock(ip, XFS_ILOCK_EXCL);
}

if (tp) {
xfs_trans_ijoin(tp, ip, lock_flags);
xfs_trans_ihold(tp, ip);
}

/*
* Truncate file. Must have write permission and not be a directory.
*/
if (mask & ATTR_SIZE) {
/*
* Only change the c/mtime if we are changing the size
* or we are explicitly asked to change it. This handles
Expand Down Expand Up @@ -410,20 +326,9 @@ xfs_setattr(
*/
xfs_iflags_set(ip, XFS_ITRUNCATED);
}
}

/*
* Change file access modes.
*/
if (mask & ATTR_MODE) {
ip->i_d.di_mode &= S_IFMT;
ip->i_d.di_mode |= iattr->ia_mode & ~S_IFMT;

inode->i_mode &= S_IFMT;
inode->i_mode |= iattr->ia_mode & ~S_IFMT;

xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
timeflags |= XFS_ICHGTIME_CHG;
} else if (tp) {
xfs_trans_ijoin(tp, ip, lock_flags);
xfs_trans_ihold(tp, ip);
}

/*
Expand Down Expand Up @@ -471,6 +376,24 @@ xfs_setattr(
timeflags |= XFS_ICHGTIME_CHG;
}

/*
* Change file access modes.
*/
if (mask & ATTR_MODE) {
umode_t mode = iattr->ia_mode;

if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
mode &= ~S_ISGID;

ip->i_d.di_mode &= S_IFMT;
ip->i_d.di_mode |= mode & ~S_IFMT;

inode->i_mode &= S_IFMT;
inode->i_mode |= mode & ~S_IFMT;

xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
timeflags |= XFS_ICHGTIME_CHG;
}

/*
* Change file access or modified times.
Expand Down

0 comments on commit c4cd747

Please sign in to comment.