linux-kernel - Re: [RFC] persistent store (version 3) (part 1 of 2)

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] [day] [month] [year] [list]

Message-ID: <20101209204708.GA2396@liondog.tnic>
Date:	Thu, 9 Dec 2010 21:47:09 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	"Luck, Tony" <tony.luck@...el.com>
Cc:	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	tglx@...utronix.de, mingo@...e.hu, greg@...ah.com,
	akpm@...ux-foundation.org, ying.huang@...el.com,
	David Miller <davem@...emloft.net>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Jim Keniston <jkenisto@...ux.vnet.ibm.com>,
	Kyungmin Park <kmpark@...radead.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [RFC] persistent store (version 3) (part 1 of 2)

On Thu, Dec 09, 2010 at 11:12:02AM -0800, Luck, Tony wrote:
> > This upsets the traditional layout of having the error
> > recovery part of the function undo all the things that
> > we did leading up to the error. Pity, because your
> > version is easier to read.
> 
> But most of your "move the error fixups to the tail, so the
> normal code path is easier to follow" bit do still hold and
> can be used.  How about this:
> 
> -Tony
> 
> ---
> 
> int pstore_mkfile(char *name, char *data, size_t size, struct timespec time,
> 		  void *private)
> {
> 	struct dentry	*root = pstore_sb->s_root;
> 	struct dentry	*dentry;
> 	struct inode	*inode;
> 	int		rc;
> 
> 	rc = -ENOMEM;
> 	inode = pstore_get_inode(pstore_sb, root->d_inode, S_IFREG | 0444, 0);
> 	if (!inode)
> 		goto fail;

I'd add a newline here.

> 	inode->i_private = private;
> 
> 	mutex_lock(&root->d_inode->i_mutex);
> 
> 	rc = -ENOSPC;
> 	dentry = d_alloc_name(root, name);
> 	if (IS_ERR(dentry))
> 		goto fail_alloc;

ditto.

> 	d_add(dentry, inode);
> 
> 	mutex_unlock(&root->d_inode->i_mutex);
> 
> 	if (!pstore_writefile(inode, dentry, data, size))
> 		goto fail_write;
> 
> 	if (time.tv_sec)
> 		inode->i_mtime = inode->i_ctime = time;
> 
> 	return 0;
> 
> fail_write:
> 	inode->i_nlink--;
> 	mutex_lock(&root->d_inode->i_mutex);
> 	d_delete(dentry);
> 	dput(dentry);
> 	mutex_unlock(&root->d_inode->i_mutex);
> 	goto fail;

Yeah, either that or add a "return rc" to save us the jmp. But since
those paths wouldn't be hit in the normal case, I don't think it matters
that much.

> 
> fail_alloc:
> 	mutex_unlock(&root->d_inode->i_mutex);
> 	iput(inode);
> 
> fail:
> 	return rc;

Thanks.

-- 
Regards/Gruss,
    Boris.
--
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/