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 <alyssa@collabora.com>
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10313>

diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
index a7e030d..143eccb 100644 (file)
--- 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 dbfce02..291e6c3 100644 (file)
--- 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);