linux-kernel - Re: [PATCH] fs/reiserfs/inode: remove dead code in _get_block_create

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: <20220726104842.flhaivkpbikqlhki@quack3>
Date:   Tue, 26 Jul 2022 12:48:42 +0200
From:   Jan Kara <jack@...e.cz>
To:     zengjx95@...il.com
Cc:     reiserfs-devel@...r.kernel.org, willy@...radead.org,
        jlayton@...nel.org, damien.lemoal@...nsource.wdc.com, jack@...e.cz,
        edward.shishkin@...il.com, linux-kernel@...r.kernel.org,
        viro@...iv.linux.org.uk, kasong@...cent.com,
        Zeng Jingxiang <linuszeng@...cent.com>
Subject: Re: [PATCH] fs/reiserfs/inode: remove dead code in
 _get_block_create_0()

On Wed 20-07-22 16:30:29, zengjx95@...il.com wrote:
> From: Zeng Jingxiang <linuszeng@...cent.com>
> 
> Since commit 27b3a5c51b50 ("kill-the-bkl/reiserfs: drop the fs race
> watchdog from _get_block_create_0()"), which removed a label that may
> have the pointer 'p' touched in its control flow, related if statements
> now eval to constant value now. Just remove them.
> 
> Assigning value NULL to p here
> 293     char *p = NULL;
> 
> In the following conditional expression, the value of p is always NULL,
> As a result, the kunmap() cannot be executed.
> 308	if (p)
> 309		kunmap(bh_result->b_page);
> 
> 355	if (p)
> 356		kunmap(bh_result->b_page);
> 
> 366	if (p)
> 367		kunmap(bh_result->b_page);
> 
> Also, the kmap() cannot be executed.
> 399	if (!p)
> 400		p = (char *)kmap(bh_result->b_page);
> 
> Signed-off-by: Zeng Jingxiang <linuszeng@...cent.com>
> Signed-off-by: Kairui Song <kasong@...cent.com>

Thanks! I've added the patch to my tree. I've also removed the unnecessary
initialization of 'p' to NULL. I'll push the patch to Linus in the coming
merge window.

									Honza

> ---
>  fs/reiserfs/inode.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> index 0cffe054b78e..fe26e1746af9 100644
> --- a/fs/reiserfs/inode.c
> +++ b/fs/reiserfs/inode.c
> @@ -305,8 +305,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
>  	result = search_for_position_by_key(inode->i_sb, &key, &path);
>  	if (result != POSITION_FOUND) {
>  		pathrelse(&path);
> -		if (p)
> -			kunmap(bh_result->b_page);
>  		if (result == IO_ERROR)
>  			return -EIO;
>  		/*
> @@ -352,8 +350,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
>  		}
>  
>  		pathrelse(&path);
> -		if (p)
> -			kunmap(bh_result->b_page);
>  		return ret;
>  	}
>  	/* requested data are in direct item(s) */
> @@ -363,8 +359,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
>  		 * when it is stored in direct item(s)
>  		 */
>  		pathrelse(&path);
> -		if (p)
> -			kunmap(bh_result->b_page);
>  		return -ENOENT;
>  	}
>  
> @@ -396,9 +390,7 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
>  	 * sure we need to.  But, this means the item might move if
>  	 * kmap schedules
>  	 */
> -	if (!p)
> -		p = (char *)kmap(bh_result->b_page);
> -
> +	p = (char *)kmap(bh_result->b_page);
>  	p += offset;
>  	memset(p, 0, inode->i_sb->s_blocksize);
>  	do {
> -- 
> 2.27.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR