RE: [PATCH v5] hfsplus: fix uninit-value by validating catalog record size

[Viacheslav Dubeyko]

On Tue, 2026-02-24 at 09:56 +0000, Charalampos Mitrodimas wrote:
> Deepanshu Kartikey <kartikey406@xxxxxxxxx> writes:
> 
> > On Tue, Feb 24, 2026 at 12:28 AM Viacheslav Dubeyko
> > <Slava.Dubeyko@xxxxxxx> wrote:
> > > 
> > 
> > > > +     case HFSPLUS_FILE_THREAD:
> > > > +             /* Ensure we have at least the fixed fields before reading nodeName.length */
> > > > +             if (fd->entrylength < offsetof(struct hfsplus_cat_thread, nodeName) +
> > > > +                 offsetof(struct hfsplus_unistr, unicode)) {
> > > > +                     pr_err("thread record too short (got %u)\n", fd->entrylength);
> > > > +                     return -EIO;
> > > > +             }
> > 
> > The check is in the HFSPLUS_FOLDER_THREAD/HFSPLUS_FILE_THREAD case in
> > hfsplus_brec_read_cat() function (fs/hfsplus/bfind.c):
> > 
> > This validates that we have at least the minimum bytes needed before
> > calling hfsplus_cat_thread_size() which reads nodeName.length.
> 
> Hi,
> 
> So... yes, while this is essentially what I recommended, just checking
> entrylength against HFSPLUS_MIN_THREAD_SZ will yield the same results,
> because:
> 
> HFSPLUS_MIN_THREAD_SZ is already defined 10, the same value as the
> offsetof chain. hfsplus_readdir() already uses it for the same
> guard. It's shorter, consistent with other places and the intent is
> immediately clear (easier to read).
> 
> 

It was my expectation to see the check with HFSPLUS_MIN_THREAD_SZ constant. And
it was the reason of my confusion. :) I completely agree with the point. It is
very important to have the clean, simple and easy understandable code.

Thanks,
Slava.