On Thu, Mar 17, 2022 at 12:20:13PM -0700, Rick Edgecombe wrote: > In fill_thread_core_info() the ptrace accessible registers are collected > to be written out as notes in a core file. The note array is allocated > from a size calculated by iterating the user regset view, and counting the > regsets that have a non-zero core_note_type. However, this only allows for > there to be non-zero core_note_type at the end of the regset view. If > there are any gaps in the middle, fill_thread_core_info() will overflow the > note allocation, as it iterates over the size of the view and the > allocation would be smaller than that. > > There doesn't appear to be any arch that has gaps such that they exceed > the notes allocation, but the code is brittle and tries to support > something it doesn't. It could be fixed by increasing the allocation size, > but instead just have the note collecting code utilize the array better. > This way the allocation can stay smaller. > > Even in the case of no arch's that have gaps in their regset views, this > introduces a change in the resulting indicies of t->notes. It does not > introduce any changes to the core file itself, because any blank notes are > skipped in write_note_info(). > > In case, the allocation logic between fill_note_info() and > fill_thread_core_info() ever diverges from the usage logic, warn and skip > writing any notes that would overflow the array. > > This fix is derrived from an earlier one[0] by Yu-cheng Yu. > > [0] https://lore.kernel.org/lkml/20180717162502.32274-1-yu-cheng.yu@xxxxxxxxx/ > > Co-developed-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > > --- > > v2: > - Warn and break out of the note collecting loop if the allocation would > overflow. Note: I tweaked it slightly to do break instead of continue > and to do it before SET_PR_FPVALID(). (Kees) This looks great; thank you for the tweak. :) Acked-by: Kees Cook <keescook@xxxxxxxxxxxx> Shall I take this separately into the for-next/execve tree, or would you rather is stay in this series? -Kees -- Kees Cook
