Re: [PATCH v3 bpf-next 01/10] lib/buildid: harden build ID parsing logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 30, 2024 at 9:05 PM Andi Kleen <ak@xxxxxxxxxxxxxxx> wrote:
>
> >       while (note_offs + sizeof(Elf32_Nhdr) < note_size) {
> >               Elf32_Nhdr *nhdr = (Elf32_Nhdr *)(note_start + note_offs);
> >
> > +             name_sz = READ_ONCE(nhdr->n_namesz);
> > +             desc_sz = READ_ONCE(nhdr->n_descsz);
> >               if (nhdr->n_type == BUILD_ID &&
> > -                 nhdr->n_namesz == sizeof("GNU") &&
> > -                 !strcmp((char *)(nhdr + 1), "GNU") &&
> > -                 nhdr->n_descsz > 0 &&
> > -                 nhdr->n_descsz <= BUILD_ID_SIZE_MAX) {
> > -                     memcpy(build_id,
> > -                            note_start + note_offs +
> > -                            ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr),
> > -                            nhdr->n_descsz);
> > -                     memset(build_id + nhdr->n_descsz, 0,
> > -                            BUILD_ID_SIZE_MAX - nhdr->n_descsz);
> > +                 name_sz == note_name_sz &&
> > +                 strcmp((char *)(nhdr + 1), note_name) == 0 &&
>
> Doesn't the strcmp need a boundary check to be inside note_size too?
>
> Other it may read into the next page, which could be unmapped, causing a fault.
> Given it's unlikely that this happen, and the end has guard pages,
> but there are some users of set_memory_np.
>
> You could just move the later checks earlier.

Yep, good catch! I'll move the overflow check and will add a note_size
check to it, thanks!

>
> The rest looks good to me.
>
> -Andi
>





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux