On Tue, Jul 09, 2024 at 01:42:42PM -0700, Andrii Nakryiko wrote:
> Harden build ID parsing logic some more, adding explicit READ_ONCE()
> when fetching values that we then use to check correctness and various
> note iteration invariants.
Just sprinkling READ_ONCE all over doesn't necessarily fix the code.
It is only needed for values that affect a loop or reference.
You have to fix stuff like this
static inline int parse_build_id(const void *page_addr,
unsigned char *build_id,
__u32 *size,
const void *note_start,
Elf32_Word note_size)
{
/* check for overflow */
if (note_start < page_addr || note_start + note_size < note_start)
^^^^^^^^^^^^^^^^^^^^^^
return -EINVAL;
which is C undefined (at least without -fwrapv-pointer) and can easily
be miscompiled if it isn't already.
I suspect the code will need more work, especially since you're
unwilling to consider any defense in depth measures.
-Andi
