linux-kernel - Re: [PATCH] Check for Null return from logfs_readpage_nolock in btree_write

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: <20140616200625.GA27360@mguzik.redhat.com>
Date:	Mon, 16 Jun 2014 22:06:26 +0200
From:	Mateusz Guzik <mguzik@...hat.com>
To:	Nicholas Krause <xerofoify@...il.com>
Cc:	joern@...fs.org, prasadjoshi.linux@...il.com, logfs@...fs.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Check for Null return from logfs_readpage_nolock in
 btree_write_block

On Mon, Jun 16, 2014 at 03:47:01PM -0400, Nicholas Krause wrote:
> Signed-off-by: Nicholas Krause <xerofoify@...il.com>
> ---
>  fs/logfs/readwrite.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/logfs/readwrite.c b/fs/logfs/readwrite.c
> index 4814031..adb9233 100644
> --- a/fs/logfs/readwrite.c
> +++ b/fs/logfs/readwrite.c
> @@ -2210,6 +2210,8 @@ void btree_write_block(struct logfs_block *block)
>  	page = logfs_get_write_page(inode, block->bix, block->level);
>  
>  	err = logfs_readpage_nolock(page);
> +	if (!err)
> +		return -ENOMEM;
>  	BUG_ON(err);
>  	BUG_ON(!PagePrivate(page));
>  	BUG_ON(logfs_block(page) != block);

This function returns 0 on success, which you turn into error condition
and return -ENOMEM.
But the function returns void, thus it cannot return the error.
It does not allocate anything, thus -ENOMEM would not be appropriate in
the first place.

Since the function returns error, nobody would check the condition in
the first place.

Even if it was not void, it would either have to return the error or
oops on BUG_ON(err).

Read the code more carefully and at least compile-test your changes.
Instructions how to compile the kernel can be found here:
http://kernelnewbies.org/FAQ/KernelCompilation

I would also suggest letting the patch wait few hours and have another
look before sending.

Cheers,
-- 
Mateusz Guzik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/