linux-kernel - Re: [PATCH] befs: redundant test on unsigned in befs_get

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: <20091016142518.998c41a2.akpm@linux-foundation.org>
Date:	Fri, 16 Oct 2009 14:25:18 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Roel Kluin <roel.kluin@...il.com>
Cc:	"Sergey S. Kostyliov" <rathamahata@...4.ru>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] befs: redundant test on unsigned in befs_get_block()?

On Fri, 16 Oct 2009 17:14:41 +0200
Roel Kluin <roel.kluin@...il.com> wrote:

> block is unsigned, this test appears redundant.
> 
> Signed-off-by: Roel Kluin <roel.kluin@...il.com>
> ---
> Or is a different test required?
> 
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 33baf27..3ab1336 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -128,13 +128,6 @@ befs_get_block(struct inode *inode, sector_t block,
>  	befs_debug(sb, "---> befs_get_block() for inode %lu, block %ld",
>  		   inode->i_ino, block);
>  
> -	if (block < 0) {
> -		befs_error(sb, "befs_get_block() was asked for a block "
> -			   "number less than zero: block %ld in inode %lu",
> -			   block, inode->i_ino);
> -		return -EIO;
> -	}
> -
>  	if (create) {
>  		befs_error(sb, "befs_get_block() was asked to write to "
>  			   "block %ld in inode %lu", block, inode->i_ino);

As far as the VFS is concerned, `block' is indeed unsigned and may well
be in the range 2G-4G with a 32-bit sector_t.  Perhaps not possible on
befs but still legal to the VFS.

So the test is wrong from that POV.

However it is possible that befs is defending itself here.  Perhaps code
internal to befs will explode if passed a "negative" block number.  Due
to coding errors within the fs implementation.

So really, we'd need to check all code paths called by
befs_get_block() and check that they are signednessly clean.

--
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/