From: Max Staudt mstaudt@suse.de
Currently, when using bochsdrm(fb), opening /dev/drm/card0 will adjust file->f_mapping, but opening /dev/fb0 will not. This may result in dangling pointers from user space when memory is evicted, and therefore in use-after-free crashes when using the emulated framebuffer device.
Bochs is the only driver to use the TTM fbdev integration (ttm_fbdev_mmap()), and thus the only one able to trigger this case. It is highlighted by the WARN_ON() in ttm_bo_vm_open() in drivers/gpu/drm/ttm/ttm_bo_vm.c which is printed upon splitting a VMA (see reproducer code below):
WARN_ON(bo->bdev->dev_mapping != vma->vm_file->f_mapping);
This happens because ttm_fbdev_mmap() sets the VMA's vm_ops pointer to point to the TTM memory handling functions, without the file's f_mapping having been adjusted. This means that file->f_mapping would still point to the address_space for the inode in the file system.
This patch avoids dangling/use-after-free pointers that remain after TTM decides to evict the memory referenced by bo->bdev->dev_mapping, as the VMAs of a mmap()ed /dev/fb0 belong to /dev/fb0 instead of the anonymous inode dev->anon_inode used by DRM. It basically mimics the adjustment of file->f_mapping that is also performed in drm_open() in drivers/gpu/drm/drm_fops.c.
For the original report, see: https://lkml.kernel.org/g/s5hy49ue4y0.wl-tiwai@suse.de
The reproducer code is as follows:
----
int main(void) { void *addr;
int fd = open("/dev/fb0", O_RDONLY); if (fd < 0) err(1, "open");
addr = mmap(0, 8192, PROT_READ, MAP_SHARED, fd, 0); if (addr == MAP_FAILED) err(1, "1st mmap");
if (mmap(addr, 4096, PROT_READ, MAP_SHARED | MAP_FIXED | MAP_ANONYMOUS, -1, 0) == MAP_FAILED) err(1, "2nd mmap");
return 0; }
----
Signed-off-by: Max Staudt mstaudt@suse.de --- drivers/gpu/drm/bochs/bochs.h | 1 + drivers/gpu/drm/bochs/bochs_fbdev.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h index 19b5ada..c37d30a 100644 --- a/drivers/gpu/drm/bochs/bochs.h +++ b/drivers/gpu/drm/bochs/bochs.h @@ -1,5 +1,6 @@ #include <linux/io.h> #include <linux/fb.h> +#include <linux/fs.h> #include <linux/console.h>
#include <drm/drmP.h> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c index 7520bf8..fd70449 100644 --- a/drivers/gpu/drm/bochs/bochs_fbdev.c +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c @@ -20,6 +20,24 @@ static int bochsfb_mmap(struct fb_info *info, return ttm_fbdev_mmap(vma, &bo->bo); }
+static int bochsfb_open_adj_file(struct fb_info *info, struct file *file) +{ + struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par; + struct bochs_device *bochs = + container_of(helper, struct bochs_device, fb.helper); + struct drm_device *dev = bochs->dev; + + /* Correct f_mapping here so it's consistent across the + * fd's lifetime. + * If this isn't done, the WARN_ON() in ttm_bo_vm_open() + * will trip upon vma_split(), hinting that other memory + * management nastiness may occur when TTM evicts. + */ + file->f_mapping = dev->anon_inode->i_mapping; + + return 0; +} + static struct fb_ops bochsfb_ops = { .owner = THIS_MODULE, .fb_check_var = drm_fb_helper_check_var, @@ -31,6 +49,7 @@ static struct fb_ops bochsfb_ops = { .fb_blank = drm_fb_helper_blank, .fb_setcmap = drm_fb_helper_setcmap, .fb_mmap = bochsfb_mmap, + .fb_open_adj_file = bochsfb_open_adj_file, };
static int bochsfb_create_object(struct bochs_device *bochs,