Yes you're right. I'm not sure that would be the best approach though
since you'd have to iterate through every function instance, and you'd
end up leaking the memory if the device was open. I'm working on
modifying the original patch to make use of the opts refcnt as well.
On Mon, Aug 21, 2017 at 11:37 PM, Manu Gautam <mgautam@xxxxxxxxxxxxxx> wrote:
> Hi,
>
>
> On 8/18/2017 11:30 AM, Manu Gautam wrote:
>> Hi,
>>
>>
>> On 8/15/2017 2:44 AM, Jerry Zhang wrote:
>
>>> @@ -1197,14 +1200,21 @@ static void f_midi_free(struct usb_function *f)
>>>
>>> midi = func_to_midi(f);
>>> opts = container_of(f->fi, struct f_midi_opts, func_inst);
>> opts could be freed as well if f_midi_free_inst already happened. Say another user
>> deleted midi instance before pcm_file was released.
>
> This would be a regression (use-after-free) with the patch.
> Do you plan to fix this as I see Felipe has already queued this for 4.14.
>
> One simple solution could be to fail midi free_instance if pcm device
> is in-use/open.
>
>
>>> - kfree(midi->id);
>>> mutex_lock(&opts->lock);
>>> - kfifo_free(&midi->in_req_fifo);
>>> - kfree(midi);
>>> - --opts->refcnt;
>>> + if (!--midi->free_ref) {
>>> + kfree(midi->id);
>>> + kfifo_free(&midi->in_req_fifo);
>>> + kfree(midi);
>>> + --opts->refcnt;
>>> + }
>>> mutex_unlock(&opts->lock);
>>> }
>>>
>>> +static void f_midi_rmidi_free(struct snd_rawmidi *rmidi)
>>> +{
>>> + f_midi_free(rmidi->private_data);
>>> +}
>>> +
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
