osdir.com
mailing list archive

Subject: [PATCH 4/7] unionfs: fix unionfs_create and unionfs_setattr to handle ATTR_KILL_S*ID - msg#00022

List: linux.file-systems.cifs

Date: Prev Next Index Thread: Prev Next Index
Don't allow either function to trip the BUG() in notify_change. For
unionfs_setattr, clear ATTR_MODE if the either ATTR_KILL_S*ID is set.

unionfs_create is setting the mode explicitly already. Don't set
ATTR_KILL_S*ID. Just fix up the mode to have the same effect. Also, move
locking the i_mutex to lower in the function. It's not needed until it
checks the i_size.

Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/unionfs/inode.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 59bb418..f2e7f25 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -78,15 +78,14 @@ static int unionfs_create(struct inode *parent, struct
dentry *dentry,
struct dentry *lower_dir_dentry;
struct iattr newattrs;

- mutex_lock(&wh_dentry->d_inode->i_mutex);
newattrs.ia_valid = ATTR_CTIME | ATTR_MODE | ATTR_ATIME
- | ATTR_MTIME | ATTR_UID | ATTR_GID | ATTR_FORCE
- | ATTR_KILL_SUID | ATTR_KILL_SGID;
+ | ATTR_MTIME | ATTR_UID | ATTR_GID | ATTR_FORCE;

- newattrs.ia_mode = mode & ~current->fs->umask;
+ newattrs.ia_mode = mode & ~(current->fs->umask|S_ISUID|S_ISGID);
newattrs.ia_uid = current->fsuid;
newattrs.ia_gid = current->fsgid;

+ mutex_lock(&wh_dentry->d_inode->i_mutex);
if (wh_dentry->d_inode->i_size != 0) {
newattrs.ia_valid |= ATTR_SIZE;
newattrs.ia_size = 0;
@@ -1032,6 +1031,9 @@ static int unionfs_setattr(struct dentry *dentry, struct
iattr *ia)
bend = dbend(dentry);
inode = dentry->d_inode;

+ if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID))
+ ia->ia_valid &= ~ATTR_MODE;
+
for (bindex = bstart; (bindex <= bend) || (bindex == bstart);
bindex++) {
lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
--
1.5.2.1

-
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html



Was this page helpful?
Yes No
Thread at a glance:

Previous Message by Date: click to view message preview

[PATCH 1/7] ecryptfs: allow lower fs to interpret ATTR_KILL_S*ID

Make sure ecryptfs doesn't trip the BUG() in notify_change. This also allows the lower filesystem to interpret these bits in their own way. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/ecryptfs/inode.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 131954b..dac4199 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -959,6 +959,14 @@ static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia) if (rc < 0) goto out; } + + /* + * mode change is for clearing setuid/setgid bits. Allow lower fs + * to interpret this in its own way. + */ + if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) + ia->ia_valid &= ~ATTR_MODE; + rc = notify_change(lower_dentry, ia); out: fsstack_copy_attr_all(inode, lower_inode, NULL); -- 1.5.2.1 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ NFS maillist - NFS@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/nfs

Next Message by Date: click to view message preview

[PATCH 2/7] knfsd: only set ATTR_KILL_S*ID if ATTR_MODE isn't being explicitly set

It's theoretically possible for a single SETATTR call to come in that sets the mode and the uid/gid. In that case, assume the mode is correct and don't set the ATTR_KILL_S*ID bits. Doing so would trip the BUG() in notify_change. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/nfsd/vfs.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 70f2c86..3b5b8cf 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -369,14 +369,19 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, if (iap->ia_valid & ATTR_MODE) { iap->ia_mode &= S_IALLUGO; imode = iap->ia_mode |= (imode & ~S_IALLUGO); + if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid) + iap->ia_valid |= ATTR_KILL_PRIV; + } else { + /* + * Revoke setuid/setgid bit on chown/chgrp, unless the mode + * is being explicitly set + */ + if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid) + iap->ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV; + if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid) + iap->ia_valid |= ATTR_KILL_SGID; } - /* Revoke setuid/setgid bit on chown/chgrp */ - if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid) - iap->ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV; - if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid) - iap->ia_valid |= ATTR_KILL_SGID; - /* Change the attributes. */ iap->ia_valid |= ATTR_CTIME; -- 1.5.2.1 - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html

Previous Message by Thread: click to view message preview

[PATCH 1/7] ecryptfs: allow lower fs to interpret ATTR_KILL_S*ID

Make sure ecryptfs doesn't trip the BUG() in notify_change. This also allows the lower filesystem to interpret these bits in their own way. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/ecryptfs/inode.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 131954b..dac4199 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -959,6 +959,14 @@ static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia) if (rc < 0) goto out; } + + /* + * mode change is for clearing setuid/setgid bits. Allow lower fs + * to interpret this in its own way. + */ + if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) + ia->ia_valid &= ~ATTR_MODE; + rc = notify_change(lower_dentry, ia); out: fsstack_copy_attr_all(inode, lower_inode, NULL); -- 1.5.2.1 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ NFS maillist - NFS@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/nfs

Next Message by Thread: click to view message preview

[PATCH 4/7] unionfs: fix unionfs_create and unionfs_setattr to handle ATTR_KILL_S*ID

Don't allow either function to trip the BUG() in notify_change. For unionfs_setattr, clear ATTR_MODE if the either ATTR_KILL_S*ID is set. This also allows the lower filesystem to interpret these bits in its own way. unionfs_create is setting the mode explicitly already, so don't set ATTR_KILL_S*ID. Just fix up the mode to have the same effect. Also, move locking the i_mutex to lower in the function. It's not needed until it checks the i_size. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/unionfs/inode.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c index 59bb418..f2e7f25 100644 --- a/fs/unionfs/inode.c +++ b/fs/unionfs/inode.c @@ -78,15 +78,14 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry, struct dentry *lower_dir_dentry; struct iattr newattrs; - mutex_lock(&wh_dentry->d_inode->i_mutex); newattrs.ia_valid = ATTR_CTIME | ATTR_MODE | ATTR_ATIME - | ATTR_MTIME | ATTR_UID | ATTR_GID | ATTR_FORCE - | ATTR_KILL_SUID | ATTR_KILL_SGID; + | ATTR_MTIME | ATTR_UID | ATTR_GID | ATTR_FORCE; - newattrs.ia_mode = mode & ~current->fs->umask; + newattrs.ia_mode = mode & ~(current->fs->umask|S_ISUID|S_ISGID); newattrs.ia_uid = current->fsuid; newattrs.ia_gid = current->fsgid; + mutex_lock(&wh_dentry->d_inode->i_mutex); if (wh_dentry->d_inode->i_size != 0) { newattrs.ia_valid |= ATTR_SIZE; newattrs.ia_size = 0; @@ -1032,6 +1031,9 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia) bend = dbend(dentry); inode = dentry->d_inode; + if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) + ia->ia_valid &= ~ATTR_MODE; + for (bindex = bstart; (bindex <= bend) || (bindex == bstart); bindex++) { lower_dentry = unionfs_lower_dentry_idx(dentry, bindex); -- 1.5.2.1 - To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
Sign up for updates to this mailing list. email:
Loading Comments...
Home | News | Patents | Sitemap | FAQ | advertise

Advertising by