linux-kernel - Re: [PATCH 16/35] Unionfs: Copyup Functionality

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: <20061208044318.GC14363@filer.fsl.cs.sunysb.edu>
Date:	Thu, 7 Dec 2006 23:43:18 -0500
From:	Josef Sipek <jsipek@....cs.sunysb.edu>
To:	Jan Engelhardt <jengelh@...ux01.gwdg.de>
Cc:	"Josef 'Jeff' Sipek" <jsipek@...sunysb.edu>,
	linux-kernel@...r.kernel.org, torvalds@...l.org, akpm@...l.org,
	hch@...radead.org, viro@....linux.org.uk,
	linux-fsdevel@...r.kernel.org, mhalcrow@...ibm.com
Subject: Re: [PATCH 16/35] Unionfs: Copyup Functionality

On Tue, Dec 05, 2006 at 10:09:19PM +0100, Jan Engelhardt wrote:
> 
> On Dec 4 2006 07:30, Josef 'Jeff' Sipek wrote:
> >+/* Determine the mode based on the copyup flags, and the existing dentry. */
> >+static int copyup_permissions(struct super_block *sb,
> >+			      struct dentry *old_hidden_dentry,
> >+			      struct dentry *new_hidden_dentry)
> >+{
> >+	struct inode *i = old_hidden_dentry->d_inode;
> 
> Screams for constification. (Or rather I do.)
 
I'm not following.
 
> >+{
> >+	int err = 0;
> >+	umode_t old_mode = old_hidden_dentry->d_inode->i_mode;
> 
> Generel question for everybody: Why do we have two same (at least on i386
> that's the case) types, umode_t and mode_t?

No idea.

> >+	} else if (S_ISBLK(old_mode)
> >+		   || S_ISCHR(old_mode)
> >+		   || S_ISFIFO(old_mode)
> >+		   || S_ISSOCK(old_mode)) {
> >+		args.mknod.parent = new_hidden_parent_dentry->d_inode;
> >+		args.mknod.dentry = new_hidden_dentry;
> >+		args.mknod.mode = old_mode;
> 
> I'd say the indent got screwed up, and it's not a mailer thing.

Right.

> >+	input_file = dentry_open(old_hidden_dentry,
> >+			unionfs_lower_mnt_idx(dentry, old_bindex), O_RDONLY | O_LARGEFILE);
> >+	if (IS_ERR(input_file)) {
...
> >+	output_file = dentry_open(new_hidden_dentry,
> >+			unionfs_lower_mnt_idx(dentry, new_bindex), O_WRONLY | O_LARGEFILE);
> 
> Here we got an 80-column buster.

/me whistles innocently

> >+	/* TODO: should we reset the error to something like -EIO? */
> 
> Handle it :)  - if it does not take a paper.

I'm not even sure if we want to reset it to begin with.

If we don't reset, the user may get some non-sensical errors, but on the
other hand, if we reset to EIO, we guarantee that the user will get a
"confusing" error message.

Josef "Jeff" Sipek.

-- 
All science is either physics or stamp collecting.
		- Ernest Rutherford
-
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/