--- a/sys/dev/misc/evdev/evdev.c +++ b/sys/dev/misc/evdev/evdev.c @@ -103,11 +103,41 @@ static void evdev_stop_repeat(struct evdev_dev *); static int evdev_check_event(struct evdev_dev *, uint16_t, uint16_t, int32_t); +/* + * evdev_master_lock serializes si_drv1 publication/teardown between + * evdev_cdev_create()/evdev_unregister() and evdev_open(). It guarantees + * that any thread which observes si_drv1 != NULL can safely take a + * reference on the evdev before the device is torn down. + * Lock order: evdev_master_lock -> evdev state lock. + */ +struct lock evdev_master_lock = LOCK_INITIALIZER("evdvmstr", 0, 0); + +void +evdev_ref(struct evdev_dev *evdev) +{ + + atomic_add_int(&evdev->ev_refs, 1); +} + +void +evdev_unref(struct evdev_dev *evdev) +{ + + KKASSERT(evdev->ev_refs > 0); + if (atomic_fetchadd_int(&evdev->ev_refs, -1) == 1) + kfree(evdev, M_EVDEV); +} + struct evdev_dev * evdev_alloc(void) { + struct evdev_dev *evdev; + + evdev = kmalloc(sizeof(struct evdev_dev), M_EVDEV, M_WAITOK | M_ZERO); + if (evdev != NULL) + evdev->ev_refs = 1; /* caller's reference */ - return kmalloc(sizeof(struct evdev_dev), M_EVDEV, M_WAITOK | M_ZERO); + return (evdev); } void @@ -116,7 +146,12 @@ if (evdev) { if (evdev->ev_cdev != NULL && evdev->ev_cdev->si_drv1 != NULL) evdev_unregister(evdev); - kfree(evdev, M_EVDEV); + /* + * Drop the allocation reference. The memory is freed only + * after the last open file descriptor (which holds its own + * reference) has been closed. + */ + evdev_unref(evdev); } } @@ -372,8 +407,16 @@ sysctl_ctx_free(&evdev->ev_sysctl_ctx); + /* + * Serialize with evdev_open(): NULL out si_drv1 and drop the cdev + * reference under evdev_master_lock, so an open that observes a + * non-NULL si_drv1 can still safely acquire its own reference. + */ + lockmgr(&evdev_master_lock, LK_EXCLUSIVE); EVDEV_LOCK(evdev); evdev->ev_cdev->si_drv1 = NULL; + evdev_unref(evdev); /* drop the cdev reference */ + /* Wake up sleepers */ LIST_FOREACH_MUTABLE(client, &evdev->ev_clients, ec_link, tmp) { evdev_revoke_client(client); @@ -386,16 +429,18 @@ } } EVDEV_UNLOCK(evdev); + lockmgr(&evdev_master_lock, LK_RELEASE); ret = evdev_cdev_destroy(evdev); evdev->ev_cdev = NULL; - if (ret == 0 && evdev->ev_lock_type == EV_LOCK_INTERNAL) { - lockuninit(&evdev->ev_mtx); - } /* * For some devices, e.g. keyboards, ev_absinfo and ev_mt * may be NULL, so check before freeing them. + * + * NOTE: the evdev structure itself (and its state lock) must stay + * valid until the last client fd is closed, so lockuninit() is + * deferred to the final kfree in evdev_unref(). */ if (evdev->ev_absinfo != NULL) evdev_free_absinfo(evdev->ev_absinfo); --- a/sys/dev/misc/evdev/cdev.c +++ b/sys/dev/misc/evdev/cdev.c @@ -99,7 +99,7 @@ evdev_open(struct dev_open_args *ap) { cdev_t dev = ap->a_head.a_dev; - struct evdev_dev *evdev = dev->si_drv1; + struct evdev_dev *evdev; struct evdev_client *client; size_t buffer_size; int ret; @@ -110,8 +110,19 @@ if (caps_priv_check_self(SYSCAP_RESTRICTEDROOT)) return (EPERM); - if (evdev == NULL) + /* + * Pin the evdev under evdev_master_lock so the sleeping allocation + * below cannot race evdev_unregister()/evdev_free() (si_drv1 is + * cleared and the cdev reference dropped under the same lock). + */ + lockmgr(&evdev_master_lock, LK_EXCLUSIVE); + evdev = dev->si_drv1; + if (evdev == NULL) { + lockmgr(&evdev_master_lock, LK_RELEASE); return (ENODEV); + } + evdev_ref(evdev); + lockmgr(&evdev_master_lock, LK_RELEASE); /* Initialize client structure */ buffer_size = evdev->ev_report_size * DEF_RING_REPORTS; @@ -152,6 +163,8 @@ if (ret != 0) { debugf(client, "cannot register evdev client"); + /* Drop the reference taken above (client leak is DF-1631) */ + evdev_unref(client->ec_evdev); } return (ret); @@ -161,14 +174,26 @@ evdev_dtor(void *data) { struct evdev_client *client = (struct evdev_client *)data; + struct evdev_dev *evdev = client->ec_evdev; - EVDEV_LOCK(client->ec_evdev); + /* + * Safe: every open file holds a reference on the evdev, so the + * structure (and its state lock) stays allocated until the last + * fd is closed even after the driver called evdev_free(). + */ + EVDEV_LOCK(evdev); if (!client->ec_revoked) - evdev_dispose_client(client->ec_evdev, client); - EVDEV_UNLOCK(client->ec_evdev); + evdev_dispose_client(evdev, client); + EVDEV_UNLOCK(evdev); funsetown(&client->ec_sigio); lockuninit(&client->ec_buffer_mtx); + /* + * Drop the reference while `client' is still allocated (the + * compiler is free to sink a load past kfree(), so the unref + * MUST NOT read client fields after the free). + */ + evdev_unref(evdev); /* may free the evdev */ kfree(client, M_EVDEV); } @@ -255,6 +280,14 @@ } while (uio->uio_resid > 0 && ret == 0) { + /* + * The device may be revoked (driver detached) mid-write; + * stop touching the evdev immediately (DF-4454). + */ + if (client->ec_revoked) { + ret = ENODEV; + break; + } ret = uiomove((void *)&event, sizeof(struct input_event), uio); if (ret == 0) ret = evdev_inject_event(evdev, event.type, event.code, @@ -716,6 +749,7 @@ if (dev != NULL) { dev->si_drv1 = evdev; + evdev_ref(evdev); /* reference owned by si_drv1 */ evdev->ev_cdev = dev; evdev->ev_unit = unit; ret = 0; --- a/sys/dev/misc/evdev/evdev_private.h +++ b/sys/dev/misc/evdev/evdev_private.h @@ -133,6 +133,9 @@ uint64_t ev_event_count; /* (s) */ uint64_t ev_report_count; /* (s) */ + /* Lifetime: reference count pinning the struct past unregister */ + volatile u_int ev_refs; /* (m) */ + /* Parent driver callbacks: */ const struct evdev_methods * ev_methods; void * ev_softc; @@ -209,6 +212,9 @@ bool evdev_event_supported(struct evdev_dev *, uint16_t); void evdev_set_abs_bit(struct evdev_dev *, uint16_t); void evdev_set_absinfo(struct evdev_dev *, uint16_t, struct input_absinfo *); +void evdev_ref(struct evdev_dev *); +void evdev_unref(struct evdev_dev *); +extern struct lock evdev_master_lock; /* Client interface: */ int evdev_register_client(struct evdev_dev *, struct evdev_client *);