Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

From: Davidlohr Bueso
Date: Mon Apr 09 2018 - 16:26:02 EST


On Mon, 09 Apr 2018, Eric Biggers wrote:

It's necessary because if we don't hold a reference to sfd->file, then it can be
a stale pointer when we compare it in __shm_open(). In particular, if the new
struct file happened to be allocated at the same address as the old one, then
'sfd->file == shp->shm_file' so the mmap would be allowed. But, it will be a
different shm segment than was intended. The caller may not even have
permissions to map it normally, yet it would be done anyway.

In the end it's just broken to have a pointer to something that can be freed out
from under you...

So this is actually handled by shm_nattch, serialized by the ipc perm->lock.
shm_destroy() is called when 0, which in turn does the fput(shm_file). Note
that shm_file is given a count of 1 when a new segment is created (deep in
get_empty_filp()). So I don't think the pointer is going anywhere, or am I missing
something?

Thanks,
Davidlohr