Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

hfs/hfsplus: avoid WARN_ON() for sanity check, use proper error handling

Commit 55d1cbbbb29e ("hfs/hfsplus: use WARN_ON for sanity check") fixed
a build warning by turning a comment into a WARN_ON(), but it turns out
that syzbot then complains because it can trigger said warning with a
corrupted hfs image.

The warning actually does warn about a bad situation, but we are much
better off just handling it as the error it is. So rather than warn
about us doing bad things, stop doing the bad things and return -EIO.

While at it, also fix a memory leak that was introduced by an earlier
fix for a similar syzbot warning situation, and add a check for one case
that historically wasn't handled at all (ie neither comment nor
subsequent WARN_ON).

Reported-by: syzbot+7bb7cd3595533513a9e7@syzkaller.appspotmail.com
Fixes: 55d1cbbbb29e ("hfs/hfsplus: use WARN_ON for sanity check")
Fixes: 8d824e69d9f3 ("hfs: fix OOB Read in __hfs_brec_find")
Link: https://lore.kernel.org/lkml/000000000000dbce4e05f170f289@google.com/
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

+10 -5
+10 -5
fs/hfs/inode.c
··· 458 458 /* panic? */ 459 459 return -EIO; 460 460 461 + res = -EIO; 461 462 if (HFS_I(main_inode)->cat_key.CName.len > HFS_NAMELEN) 462 - return -EIO; 463 + goto out; 463 464 fd.search_key->cat = HFS_I(main_inode)->cat_key; 464 465 if (hfs_brec_find(&fd)) 465 - /* panic? */ 466 466 goto out; 467 467 468 468 if (S_ISDIR(main_inode->i_mode)) { 469 - WARN_ON(fd.entrylength < sizeof(struct hfs_cat_dir)); 469 + if (fd.entrylength < sizeof(struct hfs_cat_dir)) 470 + goto out; 470 471 hfs_bnode_read(fd.bnode, &rec, fd.entryoffset, 471 472 sizeof(struct hfs_cat_dir)); 472 473 if (rec.type != HFS_CDR_DIR || ··· 480 479 hfs_bnode_write(fd.bnode, &rec, fd.entryoffset, 481 480 sizeof(struct hfs_cat_dir)); 482 481 } else if (HFS_IS_RSRC(inode)) { 482 + if (fd.entrylength < sizeof(struct hfs_cat_file)) 483 + goto out; 483 484 hfs_bnode_read(fd.bnode, &rec, fd.entryoffset, 484 485 sizeof(struct hfs_cat_file)); 485 486 hfs_inode_write_fork(inode, rec.file.RExtRec, ··· 489 486 hfs_bnode_write(fd.bnode, &rec, fd.entryoffset, 490 487 sizeof(struct hfs_cat_file)); 491 488 } else { 492 - WARN_ON(fd.entrylength < sizeof(struct hfs_cat_file)); 489 + if (fd.entrylength < sizeof(struct hfs_cat_file)) 490 + goto out; 493 491 hfs_bnode_read(fd.bnode, &rec, fd.entryoffset, 494 492 sizeof(struct hfs_cat_file)); 495 493 if (rec.type != HFS_CDR_FIL || ··· 507 503 hfs_bnode_write(fd.bnode, &rec, fd.entryoffset, 508 504 sizeof(struct hfs_cat_file)); 509 505 } 506 + res = 0; 510 507 out: 511 508 hfs_find_exit(&fd); 512 - return 0; 509 + return res; 513 510 } 514 511 515 512 static struct dentry *hfs_file_lookup(struct inode *dir, struct dentry *dentry,