check return of kmalloc()
Subsystems (2)
Hi,
I've discovered that some btrfs code doesn't check whether kmalloc() call succeeded. I poorly understand what this code does and how it can be changed, maybe it would be happy with __GFP_NOFAIL.
Also there are BUG_ON() after kmalloc()'s, if they could be changed not to panic it would be great.
tmp/cocci-output-7773-0df3b6-compression.c renamed from fs/btrfs/compression.c +0 -2
@@ -350,7 +350,6 @@ int btrfs_submit_compressed_write(structint ret;WARN_ON (start & ((u64 )PAGE_CACHE_SIZE -1 )); - cb =kmalloc (compressed_bio_size (root, compressed_len),GFP_NOFS );atomic_set (&cb->pending_bios,0 ); cb->errors =0 ; cb->inode = inode;
@@ -587,7 +586,6 @@ int btrfs_submit_compressed_read(structread_unlock (&em_tree->lock); compressed_len = em->block_len; - cb =kmalloc (compressed_bio_size (root, compressed_len),GFP_NOFS );atomic_set (&cb->pending_bios,0 ); cb->errors =0 ; cb->inode = inode;
tmp/cocci-output-7783-8f7d1b-tree-log.c renamed from fs/btrfs/tree-log.c +0 -7
@@ -336,8 +336,6 @@ static noinline int overwrite_item(strucbtrfs_release_path (root, path);return 0 ; } - dst_copy =kmalloc (item_size,GFP_NOFS ); - src_copy =kmalloc (item_size,GFP_NOFS );read_extent_buffer (eb, src_copy, src_ptr, item_size);
@@ -664,7 +662,6 @@ static noinline int drop_one_dir_item(stbtrfs_dir_item_key_to_cpu (leaf, di, &location); name_len =btrfs_dir_name_len (leaf, di); - name =kmalloc (name_len,GFP_NOFS );read_extent_buffer (leaf, name, (unsigned long )(di +1 ), name_len);btrfs_release_path (root, path);
@@ -819,7 +816,6 @@ again: ref = (struct btrfs_inode_ref *)ref_ptr; namelen =btrfs_inode_ref_name_len (eb, ref); - name =kmalloc (namelen,GFP_NOFS );BUG_ON (!name);read_extent_buffer (eb, name, (unsigned long )(ref +1 ), namelen);
@@ -1182,7 +1177,6 @@ static noinline int replay_one_name(struBUG_ON (!dir); name_len =btrfs_dir_name_len (eb, di); - name =kmalloc (name_len,GFP_NOFS ); log_type =btrfs_dir_type (eb, di);read_extent_buffer (eb, name, (unsigned long )(di +1 ), name_len);
@@ -2605,8 +2599,6 @@ static noinline int copy_items(struct btINIT_LIST_HEAD (&ordered_sums); - ins_data =kmalloc (nr *sizeof (struct btrfs_key ) + - nr *sizeof (u32),GFP_NOFS ); ins_sizes = (u32 *)ins_data; ins_keys = (struct btrfs_key *)(ins_data + nr *sizeof (u32));
tmp/cocci-output-7826-b84666-file.c renamed from fs/btrfs/file.c +0 -1
@@ -925,7 +925,6 @@ static ssize_t btrfs_file_aio_write(stru nrptrs =min ((iov_iter_count (&i) +PAGE_CACHE_SIZE -1 ) /PAGE_CACHE_SIZE ,PAGE_CACHE_SIZE / (sizeof (struct page *))); - pages =kmalloc (nrptrs *sizeof (struct page *),GFP_KERNEL );/* generic_write_checks can change our pos */ start_pos = pos;
tmp/cocci-output-7824-c1d367-inode.c renamed from fs/btrfs/inode.c +0 -3
@@ -284,7 +284,6 @@ static noinline int add_async_extent(str {struct async_extent *async_extent; - async_extent =kmalloc (sizeof (*async_extent),GFP_NOFS ); async_extent->start = start; async_extent->ram_size = ram_size; async_extent->compressed_size = compressed_size;
@@ -940,7 +939,6 @@ static int cow_file_range_async(struct iclear_extent_bit (&BTRFS_I (inode)->io_tree, start, end,EXTENT_LOCKED ,1 ,0 ,NULL ,GFP_NOFS );while (start < end) { - async_cow =kmalloc (sizeof (*async_cow),GFP_NOFS ); async_cow->inode = inode; async_cow->root = root; async_cow->locked_page = locked_page;
@@ -4891,7 +4888,6 @@ static noinline int uncompress_inline(st max_size =btrfs_file_extent_ram_bytes (leaf, item); inline_size =btrfs_file_extent_inline_item_len (leaf,btrfs_item_nr (leaf, path->slots[0 ])); - tmp =kmalloc (inline_size,GFP_NOFS ); ptr =btrfs_file_extent_inline_start (item);read_extent_buffer (leaf, tmp, ptr, inline_size);
Thanks,
Vasiliy.
Quoted 10 lines · Vasiliy Kulikov
Hi,
I've discovered that some btrfs code doesn't check whether
kmalloc()call succeeded. I poorly understand what this code does and how it can be changed, maybe it would be happy with__GFP_NOFAIL.Also there are
BUG_ON()after kmalloc()'s, if they could be changed not to panic it would be great.
Yeah. That doesn't seem right.
I have pushed a change to smatch to find places like that. It's valid to call BUG_ON() for allocation failures during init so the check is disabled by default. Use the --spammy option to enable it.
dcarpenter@bicker:~/progs/kernel/devel$ kchecker --spammy fs/btrfs/tree-log.c CHK include/linux/version.h CHK include/generated/utsrelease.h CALL scripts/checksyscalls.sh CHECK fs/btrfs/tree-log.c fs/btrfs/tree-log.c +823 add_inode_ref(43) warn: bug on allocation failure 'name' fs/btrfs/tree-log.c +868 add_inode_ref(88) warn: bug on allocation failure 'victim_name' fs/btrfs/tree-log.c +3123 btrfs_recover_log_trees(58) error: 'wc.replay_dest' dereferencing possible ERR_PTR() CC [M] fs/btrfs/tree-log.o dcarpenter@bicker:~/progs/kernel/devel$
regards,
dan carpenter