linux-kernel - Re: [patch 07/14] vfs: pass dentry to permission()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <E1Jz4n9-00079l-Ub@pomaz-ex.szeredi.hu>
Date:	Thu, 22 May 2008 09:00:47 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	viro@...IV.linux.org.uk
CC:	miklos@...redi.hu, linux-fsdevel@...r.kernel.org,
	hch@...radead.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 07/14] vfs: pass dentry to permission()

> > From: Miklos Szeredi <mszeredi@...e.cz>
> > 
> > The following patches clean up the i_op->permission() method and the
> > related VFS API.
> > 
> > Here's an overview of the changes:
> > 
> >  - ->permission() is passed a dentry instead of an inode
> >  - ->permission() is passed a integer flags parameter instead of a
> >    nameidata pointer
> 
> No.  Take a good look at the instances.
> 
> 	a) only one aberrant case cares about dentry, and for extremely
> wrong reasons.  /proc/sys/ stuff.  ecryptfs, of course, will be happy
> with any variant.

Yeah.  The real reason I changed to dentry is not /proc/sys or
ecryptfs, but because it's the Right Thing.  And if we have to touch
all instances anyway, then it basically comes for free.

Why dentry?  Because of filesystems which operate on names and not
inodes.  Currently all these manage to get by, because their
->permission() just uses the cached attributes.  But that should not
necessarily be the case.

Look at the inode_operations, each and every one passes a dentry (or
dentries) to the filesystem, except permission().

OTOH there isn't any immediate need for this, it's just an interface
rationalization thing, so...

> 	b) few flags that are looked at are trivially mapped to new MAY_...

OK.

> I have a patch series that does it, but it involves tons of fixing the
> sysctl handling to be finished ;-/  And yes, we need sysctl to quit
> doing the "I want to get ctl_table entry, so I'll do very painful search
> by dentry every damn time" in any case - look at that code, it's far
> too ugly to live.

I shut my eyes and just squinted while touching that code ;)

> 
> IOW, consider this ->permission() API change NAKed.

I'll fix b) and repost.  There's a bit more to this series, than just
the ->permission() API change.

Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/