linux-kernel - Re: xfs: commit 6552321831dc "xfs: remove i_iolock and use i_rwsem in the VFS inode instead" change causes hang

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: <20170108192634.GM1555@ZenIV.linux.org.uk>
Date:   Sun, 8 Jan 2017 19:26:34 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Christoph Hellwig <hch@....de>
Cc:     James Bottomley <James.Bottomley@...senPartnership.com>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        linux-xfs@...r.kernel.org, Dave Chinner <david@...morbit.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: xfs:  commit 6552321831dc "xfs: remove i_iolock and use i_rwsem
 in the VFS inode instead"  change causes hang

On Sun, Jan 08, 2017 at 08:09:55PM +0100, Christoph Hellwig wrote:

> No.  We need an ->ima_measure file_operation, guts of process_measurement
> turned into a library function that the FS can call after taking fs-specific
> locks.  And maybe also a small wrapper around it that takes ilock and
> can be used directly for file systems not needing special locking.

Incidentally, it had been literally years since the problems with their
pathname handling had been brought up and we *still* have got no answer.

In the current tree, ima_d_path() is quite capable of returning
path->dentry->d_name.name.  Which gets used by subsequent code,
even though there is no warranty whatsoever that it won't be
pointing to freed memory by the time the caller of ima_d_path()
gets it.

Could IMA folks be bothered to explain how the hell is that supposed to
work?  Note that the race window is *not* narrow - it includes reading the
file contents, for fuck sake!  A plenty of time for the file to be
renamed, and if the name had been long enough to be stored separately,
for the original to be freed/reused/whatnot.

Better yet, in ima_collect_measurement() they have another user of
->d_name.name, with all the same issues.