Last active
April 3, 2017 18:59
-
-
Save niklasb/416b333cb973812b39c085a42f5c19c4 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `FSEVENTS_DEVICE_FILTER_64` command for the fsevents device's `ioctl` method has a race condition bug which can lead to double `free` when the user decides to update the number of devices to 0. | |
static int | |
fseventsf_ioctl(struct fileproc *fp, u_long cmd, caddr_t data, vfs_context_t ctx) | |
{ | |
fsevent_handle *fseh = (struct fsevent_handle *)fp->f_fglob->fg_data; | |
int ret = 0; | |
fsevent_dev_filter_args64 *devfilt_args, _devfilt_args; | |
OSAddAtomic(1, &fseh->active); | |
if (fseh->flags & FSEH_CLOSING) { | |
OSAddAtomic(-1, &fseh->active); | |
return 0; | |
} | |
switch (cmd) { | |
... | |
case FSEVENTS_DEVICE_FILTER_64: | |
if (!proc_is64bit(vfs_context_proc(ctx))) { | |
ret = EINVAL; | |
break; | |
} | |
devfilt_args = (fsevent_dev_filter_args64 *)data; | |
handle_dev_filter: | |
{ | |
int new_num_devices; | |
dev_t *devices_not_to_watch, *tmp=NULL; | |
if (devfilt_args->num_devices > 256) { | |
ret = EINVAL; | |
break; | |
} | |
new_num_devices = devfilt_args->num_devices; | |
if (new_num_devices == 0) { | |
tmp = fseh->watcher->devices_not_to_watch; | |
lock_watch_table(); | |
fseh->watcher->devices_not_to_watch = NULL; | |
fseh->watcher->num_devices = new_num_devices; | |
unlock_watch_table(); | |
if (tmp) { | |
FREE(tmp, M_TEMP); | |
} | |
break; | |
} | |
... | |
The race lies in the following extract of code: | |
if (new_num_devices == 0) { | |
tmp = fseh->watcher->devices_not_to_watch; | |
lock_watch_table(); | |
fseh->watcher->devices_not_to_watch = NULL; | |
fseh->watcher->num_devices = new_num_devices; | |
unlock_watch_table(); | |
if (tmp) { | |
FREE(tmp, M_TEMP); | |
} | |
break; | |
} | |
Consider if `tmp` is fetched as a non-`NULL` pointer in the first thread, the thread is then preemted, and a second thread also fetches the same pointer into its `tmp` variable. The second thread will go on to `FREE` the pointer, and at some point later, once the first thread is resumed, it will also `FREE` this pointer. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment