I was just reading the source when I found it (no valgrind involved). So as wl_connection_destroy() already takes care of that, my patch is pointless.
My final idea would be to add support to fds that have the F_SEAL_SHRINK seal set (see memfd_create(2) and fcntl(2)), so if a client creates shm_pool with the shrink seal set, and the fd size (fstat(2)) is already >= size arg passed to wl_shm_create_pool, we no longer would need to set SIGBUS handlers when the compositor needs to access that shm memory. Does this make any sense/would be a good idea? 2016-02-19 2:49 GMT+01:00 Jonas Ådahl <jad...@gmail.com>: > On Thu, Feb 18, 2016 at 11:59:29PM +0100, Sergi Granell wrote: > > If the client passed a size <= 0 to shm_create_pool, it would > > go to err_free, which wouldn't close the fd, and thus leave it opened. > > > > We can also move the size check before the struct wl_shm_pool > > malloc, so in case the client passes a wrong size, it won't > > do an unnecessary malloc and then free. > > Did you detect this using valgrind or something like that? Because IIRC, > we collect all transmitted file descriptors and close them when the > client is destroyed (see wl_connection_destory()). Since we post an > error, the client will eventually be destroyed, so we shouldn't leak > the fd here already. > > > Jonas > > > --- > > src/wayland-shm.c | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/src/wayland-shm.c b/src/wayland-shm.c > > index a4343a4..81bf657 100644 > > --- a/src/wayland-shm.c > > +++ b/src/wayland-shm.c > > @@ -230,17 +230,17 @@ shm_create_pool(struct wl_client *client, struct > wl_resource *resource, > > { > > struct wl_shm_pool *pool; > > > > - pool = malloc(sizeof *pool); > > - if (pool == NULL) { > > - wl_client_post_no_memory(client); > > - goto err_close; > > - } > > - > > if (size <= 0) { > > wl_resource_post_error(resource, > > WL_SHM_ERROR_INVALID_STRIDE, > > "invalid size (%d)", size); > > - goto err_free; > > + goto err_close; > > + } > > + > > + pool = malloc(sizeof *pool); > > + if (pool == NULL) { > > + wl_client_post_no_memory(client); > > + goto err_close; > > } > > > > pool->refcount = 1; > > @@ -251,7 +251,7 @@ shm_create_pool(struct wl_client *client, struct > wl_resource *resource, > > wl_resource_post_error(resource, > > WL_SHM_ERROR_INVALID_FD, > > "failed mmap fd %d", fd); > > - goto err_close; > > + goto err_free; > > } > > close(fd); > > > > @@ -270,10 +270,10 @@ shm_create_pool(struct wl_client *client, struct > wl_resource *resource, > > > > return; > > > > -err_close: > > - close(fd); > > err_free: > > free(pool); > > +err_close: > > + close(fd); > > } > > > > static const struct wl_shm_interface shm_interface = { > > -- > > 2.7.1 > > > > _______________________________________________ > > wayland-devel mailing list > > wayland-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel >
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel