linux-ext4 - Re: [PATCH 0/2] fs-verity: fix !CONFIG_FS

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]

Date:   Tue, 11 Dec 2018 23:07:03 -0500
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     linux-fscrypt@...r.kernel.org,
        Chandan Rajendra <chandan@...ux.vnet.ibm.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        linux-ext4@...r.kernel.org, Tony Lindgren <tony@...mide.com>,
        Kirill Tkhai <ktkhai@...tuozzo.com>,
        linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [PATCH 0/2] fs-verity: fix !CONFIG_FS_VERITY case

On Tue, Dec 11, 2018 at 06:42:22PM -0800, Eric Biggers wrote:
> 
> Either works, but I slightly prefer my version since it minimizes the overhead
> on non-verity files when the kconfig option is enabled: it's just an i_flags
> check, rather than a function call plus an i_flags check.  The same approach is
> used in the fscrypt hooks.  Also shouldn't it be EOPNOTSUPP, not ENOTSUPP?

It's the same for both, since in the !CONFIG_FS_VERITY case the two
functions are inline functions.

There's actually a bigger issue that I've been meaning to raise, which
is that right now we'll set S_VERITY even if the VERITY feature flag
is not set.  What we should do, in my opinion, is to make ext4 fail a
r/w mount if it tries to mount the file system with the VERITY feature
flag set and the kernel is mounted with the VERITY flag set.

Also, if the kernel is compiled without CONFIG_FS_VERITY enabled,
IS_VERITY(inode) should be defined to 0, and if the an inode is found
with EXT4_VERITY_FL and the VERITY feature is not set, we should
declare the file system corrupted.

The problem is that f2fs doesn't check **any** file system features at
all.  If you mount a file system with f2fs features that the kernel
doesn't support, the f2fs file system doesn't say boo.  I suspect
hilarity would ensue if new f2fs file system with new features are
mounted on an older kernel, or a kernel where features like encryption
and verity are disabled.

So I could easily make these changes for ext4, but what we would do
for f2fs isn't clear.  I think f2fs is very broken here, but I
hesitate defining IS_VERITY(x) to 0 if !CONFIG_FS_VERITY might cause
f2fs to break even more than it does today.  I would prefer to do that
since it would remove dead code from ext4 and f2fs in the
!CONFIG_FS_VERITY case, but I think we clarify with the f2fs folks why
it is that they aren't checking the f2fs feature mask when mounting a
file system first.

       	    	      	       	    - Ted