On Tue, 21 Aug 2007 15:35:08 +1000 Timothy Shimmin <tes@xxxxxxx> wrote: > Jeff Layton wrote: > > This should fix all of the filesystems in the mainline kernels to handle > > ATTR_KILL_SUID and ATTR_KILL_SGID correctly. For most of them, this is > > just a matter of making sure that they call generic_attrkill early in > > the setattr inode op. > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/xfs/linux-2.6/xfs_iops.c | 5 ++++- > > --- a/fs/xfs/linux-2.6/xfs_iops.c > > +++ b/fs/xfs/linux-2.6/xfs_iops.c > > @@ -651,12 +651,15 @@ xfs_vn_setattr( > > struct iattr *attr) > > { > > struct inode *inode = dentry->d_inode; > > - unsigned int ia_valid = attr->ia_valid; > > + unsigned int ia_valid; > > bhv_vnode_t *vp = vn_from_inode(inode); > > bhv_vattr_t vattr = { 0 }; > > int flags = 0; > > int error; > > > > + generic_attrkill(inode->i_mode, attr); > > + ia_valid = attr->ia_valid; > > + > > if (ia_valid & ATTR_UID) { > > vattr.va_mask |= XFS_AT_UID; > > vattr.va_uid = attr->ia_uid; > > Looks reasonable to me for XFS. > Acked-by: Tim Shimmin <tes@xxxxxxx> > > So before, this clearing would happen directly in notify_change() > and now this won't happen until notify_change() calls i_op->setattr > which for a particular fs it can call generic_attrkill() to do it. > So I guess for the cases where i_op->setattr is called outside of > via notify_change, we don't normally have ATTR_KILL_SUID/SGID > set so that nothing will happen there? Right. If neither ATTR_KILL bit is set then generic_attrkill is a noop. > I guess just wondering the effect with having the code on all > setattr's. (I'm not familiar with the code path) > These bits are referenced in very few places in the current kernel tree -- mostly in the VFS layer. The *only* place I see that they actually get interpreted into a mode change is in notify_change. So places that call setattr ops w/o going through notify_change are not likely to have those bits set. But hypothetically, if a fs did set ATTR_KILL_* and call setattr directly, then the setattr would now include a mode change that clears setuid or setgid bits where it may not have before. -- Jeff Layton <jlayton@xxxxxxxxxx> - 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