linux-kernel - Re: [PATCH 1/9] vfs: add i_op->dentry

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]

Date:	Thu, 14 Mar 2013 12:15:04 +0100
From:	Miklos Szeredi <miklos@...redi.hu>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	viro@...iv.linux.org.uk, torvalds@...ux-foundation.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	hch@...radead.org, apw@...onical.com, nbd@...nwrt.org,
	neilb@...e.de, jordipujolp@...il.com, ezk@....cs.sunysb.edu,
	dhowells@...hat.com, sedat.dilek@...glemail.com,
	hooanon05@...oo.co.jp, mszeredi@...e.cz
Subject: Re: [PATCH 1/9] vfs: add i_op->dentry_open()

On Wed, Mar 13, 2013 at 11:44 PM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> On Wed, 13 Mar 2013 15:16:25 +0100 Miklos Szeredi <miklos@...redi.hu> wrote:
>
>> From: Miklos Szeredi <mszeredi@...e.cz>
>>
>> Add a new inode operation i_op->dentry_open().  This is for stacked filesystems
>> that want to return a struct file from a different filesystem.
>>
>> ...
>>
>> +/**
>> + * vfs_open - open the file at the given path
>> + * @path: path to open
>> + * @filp: newly allocated file with f_flag initialized
>> + * @cred: credentials to use
>> + */
>> +int vfs_open(const struct path *path, struct file *filp,
>> +          const struct cred *cred)
>> +{
>> +     struct inode *inode = path->dentry->d_inode;
>> +
>> +     if (inode->i_op->dentry_open)
>> +             return inode->i_op->dentry_open(path->dentry, filp, cred);
>> +     else {
>> +             filp->f_path = *path;
>> +             return do_dentry_open(filp, NULL, cred);
>> +     }
>> +}
>
> This looks like it will add some overhead to dentry_open().  That long
> walk path->dentry->d_inode->i_op->dentry_open, particularly.  Can we do
> anything?  Using filp->f_inode might save a tad.

filp->f_inode is initialized in do_dentry_open.  But we can move that
to callers.

Adding an IOP_DENTRY_OPEN flag should take care of the rest.

> It's unfortunate and a bit ugly that ->dentry_open() and
> do_dentry_open() don't have the same arguments.  One would expect and
> like them to do so, and this difference raises suspicions about design
> warts.

Hmm, the basic reason for the difference is that filesystems should
not have access to the vfsmount part of the path.

Otherwise it would be trivial to make them match up.

>
> If they _did_ have the same signature then we could perhaps populate
> ->dentry_open with do_dentry_open by default and avoid the `if'.

Except there's no way to add a default i_op, AFAIK.

>
> The test of inode->i_op->dentry_open could do with an unlikely(), but
> that won't save us :(
>
>> +EXPORT_SYMBOL(vfs_open);
>
> Did you consider EXPORT_SYMBOL_GPL()?

It is not clear to me what to base that decision on.  Either one is fine by me.

Thanks,
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/