On Mon, Jan 25, 2016 at 01:31:34PM +0300, Dan Carpenter wrote:
>
> 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.
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
>
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
