Btrfs: add support for multiple csum algorithms
Hello Josef Bacik,
The patch 607d432da054: "Btrfs: add support for multiple csum algorithms" from Dec 2, 2008, leads to the following static checker warning:
fs/btrfs/disk-io.c:320 csum_tree_block() error: __memcpy() '&found' too small (4 vs 9)
fs/btrfs/disk-io.c
278 static int csum_tree_block(struct btrfs_fs_info *fs_info,
279 struct extent_buffer *buf,
280 int verify)
281 {
282 u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
The problem here is that we wrote some incomplete stub code 8 years ago an never used it. Can we delete this stuff?
btrfs_super_csum_type() always returns 0 and thus btrfs_super_csum_size() always returns 4. There is a btrfs_set_super_csum_type() function but it is never called otherwise we would hit several bugs.
283 char *result = NULL; 284 unsigned long len; 285 unsigned long cur_len; 286 unsigned long offset = BTRFS_CSUM_SIZE; 287 char *kaddr; 288 unsigned long map_start; 289 unsigned long map_len; 290 int err; 291 u32 crc = ~(u32)0; 292 unsigned long inline_result;
Let's assume this is 64 bit system and inline_result is 8 bytes.
293
294 len = buf->len - offset;
295 while (len > 0) {
296 err = map_private_extent_buffer(buf, offset, 32,
297 &kaddr, &map_start, &map_len);
298 if (err)
299 return 1;
300 cur_len = min(len, map_len - (offset - map_start));
301 crc = btrfs_csum_data(kaddr + offset - map_start,
302 crc, cur_len);
303 len -= cur_len;
304 offset += cur_len;
305 }
306 if (csum_size > sizeof(inline_result)) {
307 result = kzalloc(csum_size, GFP_NOFS);In the future code we would allocate 9+ bytes.
308 if (!result)
309 return 1;
310 } else {
311 result = (char *)&inline_result;
312 }
313
314 btrfs_csum_final(crc, result);We only ever use the first 4 bytes. We need to add a size option to this function to support future code.
315
316 if (verify) {
317 if (memcmp_extent_buffer(buf, result, 0, csum_size)) {
318 u32 val;
319 u32 found = 0;
320 memcpy(&found, result, csum_size);If csum_size were more than 4 this would corrupt memory.
321
322 read_extent_buffer(buf, &val, 0, csum_size);
323 btrfs_warn_rl(fs_info,
324 "%s checksum verify failed on %llu wanted %X found %X "
325 "level %d",
326 fs_info->sb->s_id, buf->start,
327 val, found, btrfs_header_level(buf));
328 if (result != (char *)&inline_result)
329 kfree(result);
330 return 1;
331 }
332 } else {
333 write_extent_buffer(buf, result, 0, csum_size);
334 }
335 if (result != (char *)&inline_result)
336 kfree(result);
337 return 0;regards,
dan carpenter
Quoted 17 lines · Dan Carpenter
Hello Josef Bacik,
The patch
607d432da054: "Btrfs: add support for multiple csum algorithms" from Dec 2, 2008, leads to the following static checker warning:fs/btrfs/disk-io.c:320 csum_tree_block() error: __memcpy() '&found' too small (4 vs 9)
fs/btrfs/disk-io.c
278 static int csum_tree_block(structbtrfs_fs_info*fs_info,
279 structextent_buffer*buf,
280 int verify)281 { 282 u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
Quoted 7 lines
The problem here is that we wrote some incomplete stub code 8 years ago an never used it. Can we delete this stuff?
btrfs_super_csum_type()always returns 0 and thusbtrfs_super_csum_size()always returns 4. There is abtrfs_set_super_csum_type()function but it is never called otherwise we would hit several bugs.
Hmm..since we can only choose one type from btrfs_super_csum_type, this btrfs_set_super_csum_type() is called only when we do mkfs.btrfs.
Thanks,
-liubo
Quoted 76 lines
283 char *result = NULL; 284 unsigned long len; 285 unsigned long cur_len; 286 unsigned long offset = BTRFS_CSUM_SIZE; 287 char *kaddr; 288 unsigned long map_start; 289 unsigned long map_len; 290 int err; 291 u32 crc = ~(u32)0; 292 unsigned long inline_result;Let's assume this is 64 bit system and
inline_resultis 8 bytes.293 294 len = buf->len - offset; 295 while (len > 0) { 296 err = map_private_extent_buffer(buf, offset, 32, 297 &kaddr, &map_start, &map_len); 298 if (err) 299 return 1; 300 cur_len = min(len, map_len - (offset - map_start)); 301 crc = btrfs_csum_data(kaddr + offset - map_start, 302 crc, cur_len); 303 len -= cur_len; 304 offset += cur_len; 305 } 306 if (csum_size > sizeof(inline_result)) { 307 result = kzalloc(csum_size, GFP_NOFS);In the future code we would allocate 9+ bytes.
308 if (!result) 309 return 1; 310 } else { 311 result = (char *)&inline_result; 312 } 313 314 btrfs_csum_final(crc, result);We only ever use the first 4 bytes. We need to add a size option to this function to support future code.
315 316 if (verify) { 317 if (memcmp_extent_buffer(buf, result, 0, csum_size)) { 318 u32 val; 319 u32 found = 0; 320 memcpy(&found, result, csum_size);If
csum_sizewere more than 4 this would corrupt memory.321 322 read_extent_buffer(buf, &val, 0, csum_size); 323 btrfs_warn_rl(fs_info, 324 "%s checksum verify failed on %llu wanted %X found %X " 325 "level %d", 326 fs_info->sb->s_id, buf->start, 327 val, found, btrfs_header_level(buf)); 328 if (result != (char *)&inline_result) 329 kfree(result); 330 return 1; 331 } 332 } else { 333 write_extent_buffer(buf, result, 0, csum_size); 334 } 335 if (result != (char *)&inline_result) 336 kfree(result); 337 return 0;regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html