Re: [PATCH v5 1/8] debugfs: prevent access to possibly dead file_operations at file open

[Nicolai Stange <nicstange@gmail.com>]

Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:

> On Sun, Mar 06 2016, Nicolai Stange <nicstange@gmail.com> wrote:
>
>> + *
>> + * Allow any ongoing concurrent call into debugfs_remove() or
>> + * debugfs_remove_recursive() blocked by a former call to
>> + * debugfs_use_file_start() to proceed and return to its caller.
>> + */
>> +static void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu)
>> +{
>> +	srcu_read_unlock(&debugfs_srcu, srcu_idx);
>> +}
>> +
>> +#define F_DENTRY(filp) ((filp)->f_path.dentry)
>> +
>> +#define REAL_FOPS_DEREF(dentry)					\
>> +	((const struct file_operations *)(dentry)->d_fsdata)
>> +
>> +static int open_proxy_open(struct inode *inode, struct file *filp)
>> +{
>> +	const struct dentry *dentry = F_DENTRY(filp);
>> +	const struct file_operations *real_fops = NULL;
>> +	int srcu_idx, r;
>> +
>> +	r = debugfs_use_file_start(dentry, &srcu_idx);
>> +	if (r) {
>> +		debugfs_use_file_finish(srcu_idx);
>> +		r = -ENOENT;
>> +		goto out;
>
> this...
>
>> +out:
>> +	fops_put(real_fops);
>> +	debugfs_use_file_finish(srcu_idx);
>
> ... and that seems like an obvious double unlock?

Yes, it is. Thank you so much for reviewing and finding this!

Since v4 of this series has been applied to the driver-core tree's
driver-core-testing branch already, I'll send a patch addressing this
issue tomorrow.

Best,

Nicolai