Re: [PATCH] USB: ene_usb6250: Allocate enough memory for full object

From: Kees Cook
Date: Mon Feb 06 2023 - 13:33:51 EST


On Sat, Feb 04, 2023 at 02:43:47PM -0500, Alan Stern wrote:
> On Sat, Feb 04, 2023 at 10:35:46AM -0800, Kees Cook wrote:
> > The allocation of PageBuffer is 512 bytes in size, but the dereferencing
> > of struct ms_bootblock_idi (also size 512) happens at a calculated offset
> > within the allocation, which means the object could potentially extend
> > beyond the end of the allocation. Avoid this case by just allocating
> > enough space to catch any accesses beyond the end. Seen with GCC 13:
>
> In principle, it would be better to add a runtime check for overflow.
> Doing it this way means that the code could read an invalid value.
>
> In fact, I get the impression that this code tries to load a data
> structure which might straddle a page boundary by reading in just the
> first page. Either that, or else EntryOffset is always a multiple of
> 512 so the error cannot arise.

Yeah, I couldn't figure it out. It seems like it might move in
non-512-byte steps too sometimes? Doubling the allocation (and zero-fill
it) seemed the safest way to cover it.

-Kees

--
Kees Cook