linux-kernel - Re: [PATCH 1/3] sysfs: Remove lock ordering violation in sysfs_chmod

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:	Tue, 07 Oct 2008 14:19:41 -0700
From:	Dave Hansen <dave@...ux.vnet.ibm.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Greg KH <greg@...ah.com>, Al Viro <viro@...IV.linux.org.uk>,
	Benjamin Thery <benjamin.thery@...l.net>,
	linux-kernel@...r.kernel.org, "Serge E. Hallyn" <serue@...ibm.com>,
	Al Viro <viro@....linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH 1/3] sysfs:  Remove lock ordering violation in
	sysfs_chmod_file.

On Tue, 2008-10-07 at 03:49 -0700, Eric W. Biederman wrote:
> It is a wee bit subtle but sysfs_get_dentry grabs inode->i_mutex.
> of potentially all of the parents of sd.  So I can not hold
> the inode mutex of the directory while it is called.

Hi Eric,

This patch looks good to me.  In my quest to parse exactly what was
going on, I rewrote a description of the patch.  Could you look over
this and see if I'm on target?

--

Before this patch, inode->i_mutex is held in order to keep the inode's
mode and ctime from changing out underneath us.  If we didn't do this,
you could potentially get garbage when reading them out of the old
inode.  We calculated these new permissions once since it is redundant
to do it several times.

We also need to perform a sysfs_get_dentry() operation on the
sysfs_dirent in order to find all the dentries on each sb.  This needs
to be performed once for each sb in which the inode appears.  "[B]ut
sysfs_get_dentry grabs inode->i_mutex. of potentially all of the parents
of sd.  So I can not hold the inode mutex of the directory while it is
called."

This patch drops the inode->i_mutex over the entire "for each
sysfs_dirent" loop.  It, instead, reacquires and drops it each time we
need to calculate the new mode/ctime for the target dentries.  It does
this away from the sysfs_get_dentry() call now.

-- Dave

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