Skip to contentSkip to conversation

Btrfs: add support for multiple csum algorithms

DISCUSSION Dan Carpenter 1 participant 2 messages Jun 15, 2012 – Dec 11, 2012 Btrfs file system

lore
Share & tools
lore
#0 lore raw

Hello Josef Bacik,

The patch 607d432da054: "Btrfs: add support for multiple csum algorithms" from Dec 2, 2008, leads to the following warning:
fs/btrfs/disk-io.c:298 csum_tree_block()
error: memcpy() '&found' too small (4 vs 9)

fs/btrfs/disk-io.c
284 if (csum_size > sizeof(inline_result)) {
285 result = kzalloc(csum_size * sizeof(char), GFP_NOFS);
286 if (!result)
287 return 1;
288 } else {
289 result = (char *)&inline_result;
290 }
291
292 btrfs_csum_final(crc, result);
293
294 if (verify) {
295 if (memcmp_extent_buffer(buf, result, 0, csum_size)) {

   296                          u32 val;
   297                          u32 found = 0;
   298                          memcpy(&found, result, csum_size);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Before that commit we used to memcpy() 4 bytes and it was fine, but now csum_size can be larger than 4 bytes and there is a potential for memory corruption.

299

   300                          read_extent_buffer(buf, &val, 0, csum_size);
                                                        ^^^^

Smatch complains that "val" is too small as well.

   301                          printk_ratelimited(KERN_INFO "btrfs: %s checksum verify "
   302                                         "failed on %llu wanted %X found %X "
   303                                         "level %d\n",
   304                                         root->fs_info->sb->s_id,
   305                                         (unsigned long long)buf->start, val, found,
   306                                         btrfs_header_level(buf));
   307                          if (result != (char *)&inline_result)
   308                                  kfree(result);
   309                          return 1;

regards,
dan carpenter

#1 lore raw

This is a memory corruption bug, could someone take a look at it?

regards,
dan carpetner

Quoted 46 lines · Dan Carpenter

Hello Josef Bacik,

The patch 607d432da054: "Btrfs: add support for multiple csum algorithms" from Dec 2, 2008, leads to the following warning:
fs/btrfs/disk-io.c:298 csum_tree_block()
error: memcpy() '&found' too small (4 vs 9)

fs/btrfs/disk-io.c

   284          if (csum_size > sizeof(inline_result)) {
   285                  result = kzalloc(csum_size * sizeof(char), GFP_NOFS);
   286                  if (!result)
   287                          return 1;
   288          } else {
   289                  result = (char *)&inline_result;
   290          }
   291  
   292          btrfs_csum_final(crc, result);
   293  
   294          if (verify) {
   295                  if (memcmp_extent_buffer(buf, result, 0, csum_size)) {
   296                          u32 val;
   297                          u32 found = 0;
   298                          memcpy(&found, result, csum_size);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Before that commit we used to memcpy() 4 bytes and it was fine, but now csum_size can be larger than 4 bytes and there is a potential for memory corruption.

   299  
   300                          read_extent_buffer(buf, &val, 0, csum_size);
                                                        ^^^^

Smatch complains that "val" is too small as well.

   301                          printk_ratelimited(KERN_INFO "btrfs: %s checksum verify "
   302                                         "failed on %llu wanted %X found %X "
   303                                         "level %d\n",
   304                                         root->fs_info->sb->s_id,
   305                                         (unsigned long long)buf->start, val, found,
   306                                         btrfs_header_level(buf));
   307                          if (result != (char *)&inline_result)
   308                                  kfree(result);
   309                          return 1;

regards,
dan carpenter