Messages in this thread | | | Date | Mon, 3 Feb 2014 22:40:35 +0000 | From | Al Viro <> | Subject | Re: [PATCH v2] ceph: fix posix ACL hooks |
| |
On Mon, Feb 03, 2014 at 02:12:00PM -0800, Linus Torvalds wrote:
> >> -int afs_permission(struct inode *inode, int mask) > >> +int afs_permission(struct dentry *dentry, struct inode *inode, int mask) > > > > Oh, _lovely_. So not only do we pass dentry, the arguments are redundant > > as well. > > Note that *not* passing in inode would make the patch much bigger, > because now every filesystem would have to add the > > struct inode *inode = dentry->d_inode; > > at the top. > > Also, I'm not actually convinced it is redundant at all. Remember the > RCU lookup case? dentry->d_inode is not safe.
Umm... Point, but that actually means that we get an extra pitfall for filesystem writers here. foo_permission() passes dentry (now that it has one) to foo_wank_a_lot(), with the latter using dentry->d_inode at some point...
> >> +static int gfs2_vfs_permission(struct dentry *dentry, struct inode *inode, int mask) > >> +{ > >> + return gfs2_permission(inode, mask); > >> +} > > > > Er... You do realize that callers of gfs2_permission() tend to have > > the dentry in question, either directly or as ->d_parent of something > > they have? > > Not true. Look closer. > > Look at gfs2_lookupi() in particular, and check how it is called.
Yeowch... gfs2_ok_to_move() is particulary nasty... WTF do we need it for and why is it not racy as hell?
| |