Skip to content

Commit

Permalink
net: rfkill: reduce data->mtx scope in rfkill_fop_open
Browse files Browse the repository at this point in the history
In syzbot runs, lockdep reports that there's a (potential)
deadlock here of data->mtx being locked recursively. This
isn't really a deadlock since they are different instances,
but lockdep cannot know, and teaching it would be far more
difficult than other fixes.

At the same time we don't even really _need_ the mutex to
be locked in rfkill_fop_open(), since we're modifying only
a completely fresh instance of 'data' (struct rfkill_data)
that's not yet added to the global list.

However, to avoid any reordering etc. within the globally
locked section, and to make the code look more symmetric,
we should still lock the data->events list manipulation,
but also need to lock _only_ that. So do that.

Reported-by: syzbot+509238e523e032442b80@syzkaller.appspotmail.com
Fixes: 2c3dfba ("rfkill: sync before userspace visibility/changes")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
  • Loading branch information
Johannes Berg committed Oct 11, 2023
1 parent b2f750c commit f2ac54e
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions net/rfkill/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,6 @@ static int rfkill_fop_open(struct inode *inode, struct file *file)
init_waitqueue_head(&data->read_wait);

mutex_lock(&rfkill_global_mutex);
mutex_lock(&data->mtx);
/*
* start getting events from elsewhere but hold mtx to get
* startup events added first
Expand All @@ -1192,18 +1191,18 @@ static int rfkill_fop_open(struct inode *inode, struct file *file)
goto free;
rfkill_sync(rfkill);
rfkill_fill_event(&ev->ev, rfkill, RFKILL_OP_ADD);
mutex_lock(&data->mtx);
list_add_tail(&ev->list, &data->events);
mutex_unlock(&data->mtx);
}
list_add(&data->list, &rfkill_fds);
mutex_unlock(&data->mtx);
mutex_unlock(&rfkill_global_mutex);

file->private_data = data;

return stream_open(inode, file);

free:
mutex_unlock(&data->mtx);
mutex_unlock(&rfkill_global_mutex);
mutex_destroy(&data->mtx);
list_for_each_entry_safe(ev, tmp, &data->events, list)
Expand Down

0 comments on commit f2ac54e

Please sign in to comment.