linux-kernel - Re: [nameidata 2/2] Pass no useless nameidata to the create, lookup, and permission IOPs

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: <20070416163956.GA11001@infradead.org>
Date:	Mon, 16 Apr 2007 17:39:56 +0100
From:	Christoph Hellwig <hch@...radead.org>
To:	Andreas Gruenbacher <agruen@...e.de>
Cc:	Christoph Hellwig <hch@...radead.org>, jjohansen@...e.de,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, chrisw@...s-sol.org,
	Tony Jones <tonyj@...e.de>
Subject: Re: [nameidata 2/2] Pass no useless nameidata to the create, lookup, and permission IOPs

On Mon, Apr 16, 2007 at 06:29:20PM +0200, Andreas Gruenbacher wrote:
>  enum { MAX_NESTED_LINKS = 8 };
>  
> +/**
> + * Fields shared between nameidata and nameidata2 -- nameidata2 could
> + * be embedded in nameidata, but then the vfs code would become
> + * cluttered with dereferences.

you could use an anonymous struct embedeed in both.

> + */
> +#define __NAMEIDATA2				\
> +	struct dentry	*dentry;		\
> +	struct vfsmount *mnt;			\
> +	unsigned int	flags;			\
> +						\
> +	union {					\
> +		struct open_intent open;	\
> +	} intent;

Or better just pass argument directly.  We really
should only pass down the the dentry and the
intent to the filesystem.  The filesystem has
not business looking at mnt in the operations,
and the relevant bits of flags (mostly whether it's
O_EXCLUSIVE) should be stored in the intent, because
that's the only way it should be used.

Doing it that way also allows to fix some braindead calling conventions
like this one:

> -int permission(struct inode *inode, int mask, struct nameidata *nd)
> +int permission(struct inode *inode, int mask, struct nameidata2 *nd)
>  {
>  	umode_t mode = inode->i_mode;
>  	int retval, submask;
> @@ -278,7 +278,7 @@ int permission(struct inode *inode, int 
>   * for filesystem access without changing the "normal" uids which
>   * are used for other things.
>   */

>  static inline struct dentry *
>  do_revalidate(struct dentry *dentry, struct nameidata *nd)

Or this one.

> -			result = dir->i_op->lookup(dir, dentry, nd);
> +			result = dir->i_op->lookup(dir, dentry, ND2(nd));

or this one.

etc..
-
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/