Suspected bug in fs/ufs/inode.c - test for < 0 on unsigned sector_t- [2.6.1-rc1-mm1]

From: Jesper Juhl
Date: Sat Jan 03 2004 - 17:49:47 EST



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/