In fs/reiserfs/file.c, in reiserfs_file_write, at line 1400 in
2.6.16-rc2-mm1 we have
size_t blocks_to_allocate; /* how much blocks we need to allocate for this iteration */
size_t is an unsigned type.
Later (line 1467) we have code like:
blocks_to_allocate =
reiserfs_prepare_file_region_for_write(inode, pos,
num_pages,
write_bytes,
prepared_pages);
if (blocks_to_allocate < 0) {
res = blocks_to_allocate;
reiserfs_release_claimed_blocks(inode->i_sb,
num_pages <<
(PAGE_CACHE_SHIFT -
inode->i_blkbits));
break;
}
Spot the bug.... reiserfs_prepare_file_region_for_write can return a
negative error status, but blocks_to_allocate won't store it, and
things go wrong.
The actual result if reiserfs_prepare_file_region_for_write returns
negative is that a subsequent call to
reiserfs_allocate_blocks_for_region(&th, inode, pos,
num_pages,
write_bytes,
prepared_pages,
blocks_to_allocate);
trys to kmalloc an enormous amount of memory
allocated_blocks = kmalloc((blocks_to_allocate + will_prealloc) *
sizeof(b_blocknr_t), GFP_NOFS);
and fails so
if (res) {
reiserfs_unprepare_pages(prepared_pages, num_pages);
break;
}
which tries to unlock the pages in prepared_pages. But
reiserfs_prepare_file_region_for_write didn't leave any locked pages
in their (due to it's failure) and try_to_free_buffers BUGs out.
The "obvious" fix it to change the 'size_t' to 'ssize_t', but I'll
leave to to reiserfs-dev to create and submit a patch....
As an aside,
info gcc
tells me that '-W' will cause a warning when
* An unsigned value is compared against zero with `<' or `<='.
It doesn't :-(
NeilBrown
On Tuesday February 28, [email protected] wrote:
>
>
> As an aside,
> info gcc
> tells me that '-W' will cause a warning when
>
> * An unsigned value is compared against zero with `<' or `<='.
>
> It doesn't :-(
Actually, it does. However Linux isn't compiled with "-W".
I'm experimenting with
diff ./Makefile~current~ ./Makefile
--- ./Makefile~current~ 2006-02-28 14:44:21.000000000 +1100
+++ ./Makefile 2006-02-28 14:44:51.000000000 +1100
@@ -306,7 +306,7 @@ LINUXINCLUDE := -Iinclude \
CPPFLAGS := -D__KERNEL__ $(LINUXINCLUDE)
-CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
+CFLAGS := -W -Wall -Wno-unused -Wno-sign-compare -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common \
-ffreestanding
AFLAGS := -D__ASSEMBLY__
and it reports the following.
I think all of these are harmless except the file.c:1472 one (I
think), but it probably wouldn't hurt to tidy them up.
NeilBrown
GEN /home/src/mm-i386/Makefile
scripts/kconfig/conf -s arch/i386/Kconfig
#
# using defaults found in .config
#
SPLIT include/linux/autoconf.h -> include/config/*
Using /home/src/mm as source for kernel
GEN /home/src/mm-i386/Makefile
CHK include/linux/version.h
CC fs/reiserfs/bitmap.o
CC fs/reiserfs/do_balan.o
CC fs/reiserfs/namei.o
/home/src/mm/fs/reiserfs/namei.c: In function `reiserfs_lookup':
/home/src/mm/fs/reiserfs/namei.c:370: warning: empty body in an if-statement
/home/src/mm/fs/reiserfs/namei.c: In function `reiserfs_create':
/home/src/mm/fs/reiserfs/namei.c:629: warning: empty body in an if-statement
/home/src/mm/fs/reiserfs/namei.c:673: warning: empty body in an if-statement
/home/src/mm/fs/reiserfs/namei.c: In function `reiserfs_mknod':
/home/src/mm/fs/reiserfs/namei.c:704: warning: empty body in an if-statement
/home/src/mm/fs/reiserfs/namei.c:752: warning: empty body in an if-statement
/home/src/mm/fs/reiserfs/namei.c: In function `reiserfs_mkdir':
/home/src/mm/fs/reiserfs/namei.c:783: warning: empty body in an if-statement
/home/src/mm/fs/reiserfs/namei.c:838: warning: empty body in an if-statement
CC fs/reiserfs/inode.o
/home/src/mm/fs/reiserfs/inode.c: In function `reiserfs_get_block':
/home/src/mm/fs/reiserfs/inode.c:630: warning: comparison of unsigned expression < 0 is always false
/home/src/mm/fs/reiserfs/inode.c:942: warning: comparison of unsigned expression < 0 is always false
CC fs/reiserfs/file.o
/home/src/mm/fs/reiserfs/file.c: In function `reiserfs_file_write':
/home/src/mm/fs/reiserfs/file.c:1472: warning: comparison of unsigned expression < 0 is always false
CC fs/reiserfs/dir.o
CC fs/reiserfs/fix_node.o
/home/src/mm/fs/reiserfs/fix_node.c: In function `create_virtual_node':
/home/src/mm/fs/reiserfs/fix_node.c:196: warning: empty body in an else-statement
CC fs/reiserfs/super.o
/home/src/mm/fs/reiserfs/super.c:687: warning: missing initializer
/home/src/mm/fs/reiserfs/super.c:687: warning: (near initialization for `logging_mode[3].clrmask')
/home/src/mm/fs/reiserfs/super.c:694: warning: missing initializer
/home/src/mm/fs/reiserfs/super.c:694: warning: (near initialization for `barrier_mode[2].clrmask')
/home/src/mm/fs/reiserfs/super.c: In function `reiserfs_parse_options':
/home/src/mm/fs/reiserfs/super.c:892: warning: missing initializer
/home/src/mm/fs/reiserfs/super.c:892: warning: (near initialization for `opts[9].arg_required')
/home/src/mm/fs/reiserfs/super.c:910: warning: missing initializer
/home/src/mm/fs/reiserfs/super.c:910: warning: (near initialization for `opts[25].arg_required')
CC fs/reiserfs/prints.o
CC fs/reiserfs/objectid.o
CC fs/reiserfs/lbalance.o
CC fs/reiserfs/ibalance.o
CC fs/reiserfs/stree.o
CC fs/reiserfs/hashes.o
CC fs/reiserfs/tail_conversion.o
CC fs/reiserfs/journal.o
/home/src/mm/fs/reiserfs/journal.c: In function `journal_read':
/home/src/mm/fs/reiserfs/journal.c:2322: warning: comparison of unsigned expression >= 0 is always true
CC fs/reiserfs/resize.o
CC fs/reiserfs/item_ops.o
CC fs/reiserfs/ioctl.o
CC fs/reiserfs/procfs.o
LD fs/reiserfs/reiserfs.o
LD fs/reiserfs/built-in.o
Thanks Neil, I'll have vs evaluate and fix this.
Hans
Neil Brown wrote:
>In fs/reiserfs/file.c, in reiserfs_file_write, at line 1400 in
>2.6.16-rc2-mm1 we have
>
> size_t blocks_to_allocate; /* how much blocks we need to allocate for this iteration */
>
>size_t is an unsigned type.
>
>Later (line 1467) we have code like:
>
> blocks_to_allocate =
> reiserfs_prepare_file_region_for_write(inode, pos,
> num_pages,
> write_bytes,
> prepared_pages);
> if (blocks_to_allocate < 0) {
> res = blocks_to_allocate;
> reiserfs_release_claimed_blocks(inode->i_sb,
> num_pages <<
> (PAGE_CACHE_SHIFT -
> inode->i_blkbits));
> break;
> }
>
>
>Spot the bug.... reiserfs_prepare_file_region_for_write can return a
>negative error status, but blocks_to_allocate won't store it, and
>things go wrong.
>
>The actual result if reiserfs_prepare_file_region_for_write returns
>negative is that a subsequent call to
> reiserfs_allocate_blocks_for_region(&th, inode, pos,
> num_pages,
> write_bytes,
> prepared_pages,
> blocks_to_allocate);
>trys to kmalloc an enormous amount of memory
> allocated_blocks = kmalloc((blocks_to_allocate + will_prealloc) *
> sizeof(b_blocknr_t), GFP_NOFS);
>
>and fails so
> if (res) {
> reiserfs_unprepare_pages(prepared_pages, num_pages);
> break;
> }
>
>which tries to unlock the pages in prepared_pages. But
>reiserfs_prepare_file_region_for_write didn't leave any locked pages
>in their (due to it's failure) and try_to_free_buffers BUGs out.
>
>The "obvious" fix it to change the 'size_t' to 'ssize_t', but I'll
>leave to to reiserfs-dev to create and submit a patch....
>
>
>As an aside,
> info gcc
>tells me that '-W' will cause a warning when
>
> * An unsigned value is compared against zero with `<' or `<='.
>
>It doesn't :-(
>
>NeilBrown
>
>
>
>