linux-kernel - Re: [PATCH 27/38] union-mount: In-kernel file copyup routines

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: <20100719224152.GC11089@shell>
Date:	Mon, 19 Jul 2010 18:41:53 -0400
From:	Valerie Aurora <vaurora@...hat.com>
To:	Ian Kent <raven@...maw.net>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	Miklos Szeredi <miklos@...redi.hu>,
	Jan Blunck <jblunck@...e.de>,
	Christoph Hellwig <hch@...radead.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Valdis.Kletnieks@...edu
Subject: Re: [PATCH 27/38] union-mount: In-kernel file copyup routines

On Tue, Jul 13, 2010 at 12:56:20PM +0800, Ian Kent wrote:
> On Tue, Jun 15, 2010 at 11:39:57AM -0700, Valerie Aurora wrote:
> > When a file on the read-only layer of a union mount is altered, it
> > must be copied up to the topmost read-write layer.  This patch creates
> > union_copyup() and its supporting routines.
> > 
> > Thanks to Valdis Kletnieks for a bug fix.
> > 
> > Cc: Valdis.Kletnieks@...edu
> > ---
> >  fs/union.c |  323 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/union.h |    7 +-
> >  2 files changed, 329 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/union.c b/fs/union.c
> > index 76a6c34..0982446 100644
> > --- a/fs/union.c
> > +++ b/fs/union.c
> > +/**
> > + * do_union_copyup_len - Copy up a file given its path (and its parent's)
> > + *
> > + * @nd: nameidata for topmost parent dir
> > + * @path: path of file to be copied up
> > + * @copy_all: if set, copy all of the file's data and ignore @len
> > + * @len: if @copy_all is not set, number of bytes of file data to copy up
> > + *
> > + * Newly copied up path is returned in @path.
> > + */
> > +
> > +static int do_union_copyup_len(struct nameidata *nd, struct path *path,
> > +			       int copy_all, size_t len)
> > +{
> > +	struct path *parent = &nd->path;
> > +	int error;
> > +
> > +	if (!IS_DIR_UNIONED(parent->dentry))
> > +		return 0;
> > +	if (parent->mnt == path->mnt)
> > +		return 0;
> > +	if (!S_ISREG(path->dentry->d_inode->i_mode) &&
> > +	    !S_ISLNK(path->dentry->d_inode->i_mode))
> > +		return 0;
> > +
> > +	BUG_ON(!S_ISDIR(parent->dentry->d_inode->i_mode));
> > +
> > +	mutex_lock(&parent->dentry->d_inode->i_mutex);
> > +	error = -ENOENT;
> > +	if (IS_DEADDIR(parent->dentry->d_inode))
> > +		goto out_unlock;
> > +
> > +	if (copy_all && S_ISREG(path->dentry->d_inode->i_mode)) {
> > +		error = -EFBIG;
> > +		len = i_size_read(path->dentry->d_inode);
> > +		if (((size_t)len != len) || ((ssize_t)len != len))
> > +			goto out_unlock;
> 
> OK, call me dumb, but what does this comparison of len to len do?

It checks if len (the size of the file to be copied up) will overflow
size_t or ssize_t on this machine.  The file could have been created
on a 64-bit box, and be too big to be manipulated on a 32-bit box.  It
could use a comment, fixed.

-VAL
--
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/