Re: [PATCH v3 RESEND] hfs: validate catalog CNIDs before instantiating inodes

[David Maximiliano Hermitte <davemadmaxxx@gmail.com>]

Hi Slava,

Thank you for the review.

Yes, I agree that the initial length validation can be simplified to:

        rec_len = fd->entrylength;
        if (rec_len <= 0 || rec_len > sizeof(rec))
                return -EIO;

I also agree that, for the fixed-size file and directory catalog records,
the checks should require the exact record size:

        rec_len != sizeof(struct hfs_cat_file)

and:

        rec_len != sizeof(struct hfs_cat_dir)

Regarding hfs_write_inode(), I intentionally left it unchanged because
the patch was trying to reject invalid CNIDs at the catalog lookup and
inode-instantiation boundaries, before a corrupted inode could reach
writeback.

However, I agree that an additional validation there may be useful as
defense in depth. Would you prefer hfs_is_valid_cnid() to be checked in
hfs_write_inode() as well, and should that check precede the existing
BUG path while otherwise leaving its behavior unchanged?

I will prepare the next revision after your guidance on that point.

Thanks,
David