From b1d4def05992bf061d6e0cc901ca00b7995e8d75 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 12 Mar 2014 22:05:15 -0400 Subject: nouveau: safen up nouveau_device list usage against concurrent access I cannot make nouveau_bo_wrap thread-safe (by design), but it seems to be used to convert drm fb's to nouveau_bo's and to get a notify handle from fifo->notify in nv30_screen.c Signed-off-by: Ilia Mirkin Signed-off-by: Maarten Lankhorst --- nouveau/nouveau.c | 108 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 87 insertions(+), 21 deletions(-) (limited to 'nouveau/nouveau.c') diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c index ee7893b3..75dfb0ed 100644 --- a/nouveau/nouveau.c +++ b/nouveau/nouveau.c @@ -85,6 +85,12 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev) if (!nvdev) return -ENOMEM; + ret = pthread_mutex_init(&nvdev->lock, NULL); + if (ret) { + free(nvdev); + return ret; + } + nvdev->base.fd = fd; ver = drmGetVersion(fd); @@ -161,6 +167,7 @@ nouveau_device_del(struct nouveau_device **pdev) if (nvdev->close) drmClose(nvdev->base.fd); free(nvdev->client); + pthread_mutex_destroy(&nvdev->lock); free(nvdev); *pdev = NULL; } @@ -191,6 +198,8 @@ nouveau_client_new(struct nouveau_device *dev, struct nouveau_client **pclient) int id = 0, i, ret = -ENOMEM; uint32_t *clients; + pthread_mutex_lock(&nvdev->lock); + for (i = 0; i < nvdev->nr_client; i++) { id = ffs(nvdev->client[i]) - 1; if (id >= 0) @@ -199,7 +208,7 @@ nouveau_client_new(struct nouveau_device *dev, struct nouveau_client **pclient) clients = realloc(nvdev->client, sizeof(uint32_t) * (i + 1)); if (!clients) - return ret; + goto unlock; nvdev->client = clients; nvdev->client[i] = 0; nvdev->nr_client++; @@ -214,6 +223,9 @@ out: } *pclient = &pcli->base; + +unlock: + pthread_mutex_unlock(&nvdev->lock); return ret; } @@ -225,7 +237,9 @@ nouveau_client_del(struct nouveau_client **pclient) if (pcli) { int id = pcli->base.id; nvdev = nouveau_device(pcli->base.device); + pthread_mutex_lock(&nvdev->lock); nvdev->client[id / 32] &= ~(1 << (id % 32)); + pthread_mutex_unlock(&nvdev->lock); free(pcli->kref); free(pcli); } @@ -331,12 +345,43 @@ nouveau_object_find(struct nouveau_object *obj, uint32_t pclass) static void nouveau_bo_del(struct nouveau_bo *bo) { + struct nouveau_device_priv *nvdev = nouveau_device(bo->device); struct nouveau_bo_priv *nvbo = nouveau_bo(bo); struct drm_gem_close req = { bo->handle }; - DRMLISTDEL(&nvbo->head); + + pthread_mutex_lock(&nvdev->lock); + if (nvbo->name) { + if (atomic_read(&bo->refcnt)) { + /* + * bo has been revived by a race with + * nouveau_bo_prime_handle_ref, or nouveau_bo_name_ref. + * + * In theory there's still a race possible with + * nouveau_bo_wrap, but when using this function + * the lifetime of the handle is probably already + * handled in another way. If there are races + * you're probably using nouveau_bo_wrap wrong. + */ + pthread_mutex_unlock(&nvdev->lock); + return; + } + DRMLISTDEL(&nvbo->head); + /* + * This bo has to be closed with the lock held because gem + * handles are not refcounted. If a shared bo is closed and + * re-opened in another thread a race against + * DRM_IOCTL_GEM_OPEN or drmPrimeFDToHandle might cause the + * bo to be closed accidentally while re-importing. + */ + drmIoctl(bo->device->fd, DRM_IOCTL_GEM_CLOSE, &req); + pthread_mutex_unlock(&nvdev->lock); + } else { + DRMLISTDEL(&nvbo->head); + pthread_mutex_unlock(&nvdev->lock); + drmIoctl(bo->device->fd, DRM_IOCTL_GEM_CLOSE, &req); + } if (bo->map) munmap(bo->map, bo->size); - drmIoctl(bo->device->fd, DRM_IOCTL_GEM_CLOSE, &req); free(nvbo); } @@ -363,15 +408,17 @@ nouveau_bo_new(struct nouveau_device *dev, uint32_t flags, uint32_t align, return ret; } + pthread_mutex_lock(&nvdev->lock); DRMLISTADD(&nvbo->head, &nvdev->bo_list); + pthread_mutex_unlock(&nvdev->lock); *pbo = bo; return 0; } int -nouveau_bo_wrap(struct nouveau_device *dev, uint32_t handle, - struct nouveau_bo **pbo) +nouveau_bo_wrap_locked(struct nouveau_device *dev, uint32_t handle, + struct nouveau_bo **pbo) { struct nouveau_device_priv *nvdev = nouveau_device(dev); struct drm_nouveau_gem_info req = { .handle = handle }; @@ -404,6 +451,18 @@ nouveau_bo_wrap(struct nouveau_device *dev, uint32_t handle, return -ENOMEM; } +int +nouveau_bo_wrap(struct nouveau_device *dev, uint32_t handle, + struct nouveau_bo **pbo) +{ + struct nouveau_device_priv *nvdev = nouveau_device(dev); + int ret; + pthread_mutex_lock(&nvdev->lock); + ret = nouveau_bo_wrap_locked(dev, handle, pbo); + pthread_mutex_unlock(&ndev->lock); + return ret; +} + int nouveau_bo_name_ref(struct nouveau_device *dev, uint32_t name, struct nouveau_bo **pbo) @@ -413,18 +472,21 @@ nouveau_bo_name_ref(struct nouveau_device *dev, uint32_t name, struct drm_gem_open req = { .name = name }; int ret; + pthread_mutex_lock(&nvdev->lock); DRMLISTFOREACHENTRY(nvbo, &nvdev->bo_list, head) { if (nvbo->name == name) { *pbo = NULL; nouveau_bo_ref(&nvbo->base, pbo); + pthread_mutex_unlock(&nvdev->lock); return 0; } } ret = drmIoctl(dev->fd, DRM_IOCTL_GEM_OPEN, &req); if (ret == 0) { - ret = nouveau_bo_wrap(dev, req.handle, pbo); + ret = nouveau_bo_wrap_locked(dev, req.handle, pbo); nouveau_bo((*pbo))->name = name; + pthread_mutex_unlock(&nvdev->lock); } return ret; @@ -435,13 +497,16 @@ nouveau_bo_name_get(struct nouveau_bo *bo, uint32_t *name) { struct drm_gem_flink req = { .handle = bo->handle }; struct nouveau_bo_priv *nvbo = nouveau_bo(bo); - if (!nvbo->name) { + + *name = nvbo->name; + if (!*name || *name == ~0) { int ret = drmIoctl(bo->device->fd, DRM_IOCTL_GEM_FLINK, &req); - if (ret) + if (ret) { + *name = 0; return ret; - nvbo->name = req.name; + } + nvbo->name = *name = req.name; } - *name = nvbo->name; return 0; } @@ -466,19 +531,18 @@ nouveau_bo_prime_handle_ref(struct nouveau_device *dev, int prime_fd, int ret; unsigned int handle; - ret = drmPrimeFDToHandle(dev->fd, prime_fd, &handle); - if (ret) { - nouveau_bo_ref(NULL, bo); - return ret; - } + nouveau_bo_ref(NULL, bo); - ret = nouveau_bo_wrap(dev, handle, bo); - if (ret) { - nouveau_bo_ref(NULL, bo); - return ret; + pthread_mutex_lock(&nvdev->lock); + ret = drmPrimeFDToHandle(dev->fd, prime_fd, &handle); + if (ret == 0) { + ret = nouveau_bo_wrap_locked(dev, handle, bo); + if (!bo->name) + // XXX: Force locked DRM_IOCTL_GEM_CLOSE to rule out race conditions + bo->name = ~0; } - - return 0; + pthread_mutex_unlock(&nvdev->lock); + return ret; } int @@ -490,6 +554,8 @@ nouveau_bo_set_prime(struct nouveau_bo *bo, int *prime_fd) ret = drmPrimeHandleToFD(bo->device->fd, nvbo->base.handle, DRM_CLOEXEC, prime_fd); if (ret) return ret; + if (!nvbo->name) + nvbo->name = ~0; return 0; } -- cgit v1.2.3