DF-4454 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 | --- 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 *); |