linux-ext4 - Re: [PATCH 1/2] ext4: fix error handling in ext4_end_enable

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: <YEo4958DZYm/21b+@mit.edu>
Date:   Thu, 11 Mar 2021 10:36:23 -0500
From:   "Theodore Ts'o" <tytso@....edu>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-fscrypt@...r.kernel.org, Yunlei He <heyunlei@...onor.com>,
        bintian.wang@...onor.com, stable@...r.kernel.org
Subject: Re: [PATCH 1/2] ext4: fix error handling in ext4_end_enable_verity()

On Tue, Mar 02, 2021 at 12:04:19PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@...gle.com>
> 
> ext4 didn't properly clean up if verity failed to be enabled on a file:
> 
> - It left verity metadata (pages past EOF) in the page cache, which
>   would be exposed to userspace if the file was later extended.
> 
> - It didn't truncate the verity metadata at all (either from cache or
>   from disk) if an error occurred while setting the verity bit.
> 
> Fix these bugs by adding a call to truncate_inode_pages() and ensuring
> that we truncate the verity metadata (both from cache and from disk) in
> all error paths.  Also rework the code to cleanly separate the success
> path from the error paths, which makes it much easier to understand.
> 
> Reported-by: Yunlei He <heyunlei@...onor.com>
> Fixes: c93d8f885809 ("ext4: add basic fs-verity support")
> Cc: <stable@...r.kernel.org> # v5.4+
> Signed-off-by: Eric Biggers <ebiggers@...gle.com>

Thanks, LGTM.

I've applied this to the ext4 with minor adjustment; I eliminated the
double blank line here:

> +	ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS);
> +	return 0;
> +
> +
> +stop_and_cleanup:
> +	ext4_journal_stop(handle);
     ...

						- Ted