On Mon, Jun 10, 2024 at 10:54:26PM +0200, Vlastimil Babka wrote: > On 6/10/24 7:07 PM, Christoph Lameter (Ampere) wrote: > > On Fri, 7 Jun 2024, Chengming Zhou wrote: > > > >> There are two inconsistencies in check_object(), which are alignment > >> padding checking and object padding checking. We only print the error > >> messages but don't return 0 to tell callers that something is wrong > >> and needs to be handled. Please see alloc_debug_processing() and > >> free_debug_processing() for details. > > > > If the error is in the padding and the redzones are ok then its likely > > that the objects are ok. So we can actually continue with this slab page > > instead of isolating it. > > > > We isolate it in the case that the redzones have been violated because > > that suggests someone overwrote the end of the object f.e. In that case > > objects may be corrupted. Its best to isolate the slab and hope for the > > best. > > > > If it was just the padding then the assumption is that this may be a > > scribble. So clean it up and continue. "a scribble"? :P If padding got touched, something has the wrong size for an object write. It should be treated just like the redzones. We want maximal coverage if this checking is enabled. > Hm is it really worth such nuance? We enabled debugging and actually hit a > bug. I think it's best to keep things as much as they were and not try to > allow further changes. This e.g. allows more detailed analysis if somebody > later notices the bug report and decides to get a kdump crash dump (or use > drgn on live system). Maybe we should even stop doing the restore_bytes() > stuff, and prevent any further frees in the slab page to happen if possible > without affecting fast paths (now we mark everything as used but don't > prevent further frees of objects that were actually allocated before). > > Even if some security people enable parts of slub debugging for security > people it is my impression they would rather panic/reboot or have memory > leaked than trying to salvage the slab page? (CC Kees) Yeah, if we're doing these checks, we should do the checks fully. Padding is just extra redzone. :) -- Kees Cook
