[alsa-devel] [PATCH v2 1/2] ALSA: fireworks: accessing to user space outside spinlock

[Takashi Sakamoto]

On Aug 30 2016 15:07, Vaishali Thakkar wrote:
>>> diff --git a/sound/firewire/fireworks/fireworks_hwdep.c b/sound/firewire/fireworks/fireworks_hwdep.c
>>> index 33df865..8ed4b29 100644
>>> --- a/sound/firewire/fireworks/fireworks_hwdep.c
>>> +++ b/sound/firewire/fireworks/fireworks_hwdep.c
>>> @@ -38,6 +38,7 @@ hwdep_read_resp_buf(struct snd_efw *efw, char __user *buf, long remained,
>>>  	buf += sizeof(type);
>>>
>>>  	/* write into buffer as many responses as possible */
>>> +	spin_lock_irq(&efw->lock);
>>>  	while (efw->resp_queues > 0) {
>>>  		t = (struct snd_efw_transaction *)(efw->pull_ptr);
>>>  		length = be32_to_cpu(t->length) * sizeof(__be32);
>>> @@ -52,9 +53,13 @@ hwdep_read_resp_buf(struct snd_efw *efw, char __user *buf, long remained,
>>>  				(unsigned int)(efw->pull_ptr - efw->resp_buf);
>>>  			till_end = min_t(unsigned int, length, till_end);
>>>
>>> +			spin_unlock_irq(&efw->lock);
>>> +
>>>  			if (copy_to_user(buf, efw->pull_ptr, till_end))
>>>  				return -EFAULT;
>>>
>>> +			spin_lock_irq(&efw->lock);
>>> +
>>>  			efw->pull_ptr += till_end;
>>
>> Strictly speaking, this is still racy.  efw->pull_ptr is accessed
>> outside the spinlock, and it can be modified while copy_to_user() is
>> being processed.
>
> Should disabling pagefaults while interacting with user space work here?

Instead, assignment efw->pull_ptr to automatic variable in the spin 
lock, then use it to copy_to_user(). The pointer is in ring buffer and 
moves when receiving replies from actual devices, thus it's still valid 
as long as an application sent transactions in order.
(One application is allowed to use this interface in a time.)

Not the best, but better with less complexities, I think.


Regards

Takashi Sakamoto