On Wed, Feb 13, 2019 at 10:12 PM Qian Cai <[email protected]> wrote: > > On Wed, 2019-02-13 at 11:31 +0100, Andrey Konovalov wrote: > > On Wed, Feb 13, 2019 at 3:06 AM Qian Cai <[email protected]> wrote: > > > > > > get_freepointer() could return NULL if there is no more free objects in > > > the slab. However, it could return a tagged pointer (like > > > 0x2200000000000000) with KASAN_SW_TAGS which would escape the NULL > > > object checking in check_valid_pointer() and trigger errors below, so > > > untag the object before checking for a NULL object there. > > > > I think this solution is just masking the issue. get_freepointer() > > shouldn't return tagged NULLs. Apparently when we save a freelist > > pointer, the object where the pointer gets written is tagged > > differently, than this same object when the pointer gets read. I found > > one case where this happens (the last patch out my 5 patch series), > > but apparently there are more. > > Well, the problem is that, > > __free_slab > for_each_object(p, s, page_address(page) [1] > check_object(s, page, p ...) > get_freepointer(s, p) > > [1]: p += s->size > > page_address() tags the address using page_kasan_tag(page), so each "p" here > has > that tag.
Ah, I see what the issue is. With those 5 patches page_address() should return 0xff-tagged pointer here, but when we set_freepointer() the tag indeed might be different. OK, I think that patch that you linked below is the better way to deal with this. I've added a detailed comment to it and sent it. Thanks! > > However, at beginning in allocate_slab(), it tags each object with a random > tag, > and then calls set_freepointer(s, p, NULL) > > As the result, get_freepointer() returns a tagged NULL because it never be > able > to obtain the original tag of the object anymore, and this calculation is now > wrong. > > return (void *)((unsigned long)ptr ^ s->random ^ ptr_addr); > > This also explain why this patch also works, as it unifies the tags. > > https://marc.info/?l=linux-mm&m=154955366113951&w=2 > >
