From fcebdcb3b39f05583d6db45dca453c6aef3f17bb Mon Sep 17 00:00:00 2001 From: "Carsten Haitzler (Rasterman)" Date: Mon, 19 Apr 2021 09:36:47 +0100 Subject: [PATCH] panfrost: Fix Bo imports to not take the process down if fd is invalid If the calling process happens to use an invalid bo (e.g. fd has been closed), this led to lseek() returning -1 and this mmap trying to mmap a buffer of size -1 (0xffffffffffffffff ...) which led to mmap failing, which led to munmap failing which then led to the process aborting. This fixes that to gracefully handle the mmap failures and percolate the failures back up through the API so eglCreateImage will not return a valid image anymore, thus the error is detectable in the caller process too. Reviewed-by: Alyssa Rosenzweig Reviewed-by: Tomeu Vizoso Part-of: --- src/gallium/drivers/panfrost/pan_resource.c | 7 +++++++ src/panfrost/lib/pan_bo.c | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index a7e030d8b98..143eccbd507 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -103,6 +103,13 @@ panfrost_resource_from_handle(struct pipe_screen *pscreen, } rsc->image.data.bo = panfrost_bo_import(dev, whandle->handle); + /* Sometimes an import can fail e.g. on an invalid buffer fd, out of + * memory space to mmap it etc. + */ + if (!rsc->image.data.bo) { + ralloc_free(rsc); + return NULL; + } if (rsc->image.layout.crc_mode == PAN_IMAGE_CRC_OOB) rsc->image.crc.bo = panfrost_bo_create(dev, rsc->image.layout.crc_size, 0); diff --git a/src/panfrost/lib/pan_bo.c b/src/panfrost/lib/pan_bo.c index dbfce02e519..291e6c347f5 100644 --- a/src/panfrost/lib/pan_bo.c +++ b/src/panfrost/lib/pan_bo.c @@ -335,8 +335,11 @@ panfrost_bo_mmap(struct panfrost_bo *bo) bo->ptr.cpu = os_mmap(NULL, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED, bo->dev->fd, mmap_bo.offset); if (bo->ptr.cpu == MAP_FAILED) { - fprintf(stderr, "mmap failed: %p %m\n", bo->ptr.cpu); - assert(0); + bo->ptr.cpu = NULL; + fprintf(stderr, + "mmap failed: result=%p size=0x%llx fd=%i offset=0x%llx %m\n", + bo->ptr.cpu, (long long)bo->size, bo->dev->fd, + (long long)mmap_bo.offset); } } @@ -472,9 +475,16 @@ panfrost_bo_import(struct panfrost_device *dev, int fd) bo->dev = dev; bo->ptr.gpu = (mali_ptr) get_bo_offset.offset; bo->size = lseek(fd, 0, SEEK_END); + /* Sometimes this can fail and return -1. size of -1 is not + * a nice thing for mmap to try mmap. Be more robust also + * for zero sized maps and fail nicely too + */ + if ((bo->size == 0) || (bo->size == (size_t)-1)) { + pthread_mutex_unlock(&dev->bo_map_lock); + return NULL; + } bo->flags = PAN_BO_SHARED; bo->gem_handle = gem_handle; - assert(bo->size > 0); p_atomic_set(&bo->refcnt, 1); // TODO map and unmap on demand? panfrost_bo_mmap(bo);