linux-ext4 - Re: [PATCH v4 6/6] ext4: prevent getting empty inode buffer

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: <YS2brqdSIO8mQs3U@mit.edu>
Date:   Mon, 30 Aug 2021 23:02:06 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Zhang Yi <yi.zhang@...wei.com>
Cc:     linux-ext4@...r.kernel.org, adilger.kernel@...ger.ca, jack@...e.cz,
        yukuai3@...wei.com
Subject: Re: [PATCH v4 6/6] ext4: prevent getting empty inode buffer

On Thu, Aug 26, 2021 at 09:04:12PM +0800, Zhang Yi wrote:
> 
> So this patch initialize the inode buffer by filling the in-mem inode
> contents if we skip read I/O, ensure that the buffer is really uptodate.
> 
> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
> ---
>  fs/ext4/inode.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 3c36e701e30e..8b37f55b04ad 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4446,8 +4446,8 @@ static int ext4_fill_raw_inode(struct inode *inode, struct ext4_inode *raw_inode
>   * inode.
>   */
>  static int __ext4_get_inode_loc(struct super_block *sb, unsigned long ino,
> -				struct ext4_iloc *iloc, int in_mem,
> -				ext4_fsblk_t *ret_block)
> +				struct inode *inode, struct ext4_iloc *iloc,
> +				int in_mem, ext4_fsblk_t *ret_block)


In this patch you've added a new argument 'inode'.  However, if in_mem
is true, and inode is NULL, the kernel will crash with a null pointer
dereference.  Furthermore, whenever in_mem is false, the callers pass
in NULL for inode.

Given that, perhaps we should just drop the in_mem argument, and then
instead of

	if (in_mem) {

we do:

	if (inode && !ext4_test_inode_state(inode, EXT4_STATE_XATTR) {

with the comments adjusted accordingly?

I think it will make the code a bit simpler and readable.

What do you think?

					- Ted