Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
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