Re: Suspected bug in fs/ufs/inode.c and other filesystems - testfor < 0 on unsigned sector_t - [2.6.1-rc1-mm1]

[Jesper Juhl]

Hmm, the exact same thing as below seems to be going on in

fs/adfs/inode.c		- line 30
fs/befs/linuxvfs.c	- line 135
fs/bfs/file.c		- line 67 (this one's a little different, but the sector_t < 0 check is involved
fs/reiserfs/inode.c	- line 577

should I be worried?  Is this worth digging into any further?
I don't mind doing some intensive fs code studying, but before I commit a
lot of time to this it would be nice with a confirmation from someone who
already knows this stuff that it's worth spending time on... I don't yet
know enough to judge that..


/Jesper Juhl


On Sat, 3 Jan 2004, Jesper Juhl wrote:

>
> Hi,
>
> I think I may have found a bug in fs/ufs/inode.c
>
> lines 334 & 335 have this code (in function ufs_getfrag_block) :
>
> 	if (fragment < 0)
> 		goto abort_negative;
>
> fragment is an argument to ufs_getfrag_block of type sector_t
> digging a little in include/linux and include/asm reveals sector_t to be
> an an unsigned type in all archs (at least as far as I've been able to
> determine).
>
> include/linux/types.h defines sector_t as
>
> typedef unsigned long sector_t;
>
> and in include/asm* sector_t is defined as :
>
> asm-sh/types.h:typedef u64 sector_t;
> asm-x86_64/types.h:typedef u64 sector_t;
> asm-h8300/types.h:typedef u64 sector_t;
> asm-i386/types.h:typedef u64 sector_t;
> asm-mips/types.h:typedef u64 sector_t;
> asm-s390/types.h:typedef u64 sector_t;
> asm-ppc/types.h:typedef u64 sector_t;
>
> all unsigned types.
>
> So, since sector_t is unsigned it can never be less than zero, hence the
> branch on line 334 will never be taken and the assiciated label
> "abort_negative" will never be reached (and it's not referenced anywhere
> else).
>
> I don't know enough about UFS to be able to tell if this is actually a
> problem or not, but I suspect that it might be.. If it's not a problem,
> then the code that tests for (fragment < 0) and the associated code
> in abort_negative might as well be removed.
>
> Is this analysis correct?  If it is, can the code simply be removed?
>
>
> Kind regards,
>
> Jesper Juhl
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/