Hi,
On 8/15/2017 2:44 AM, Jerry Zhang wrote:
> Currenly, f_midi_free uses snd_card_free, which will wait
> until the user has released the sound card before
> returning. However, if the user doesn't release the sound
> card, then f_midi_free can block for an arbitrary amount
> of time, which also blocks any gadget operations on that
> thread.
>
> Instead, we can use snd_card_free_when_closed which returns
> before all handles are released. Since f_midi can be
> accessed through rmidi if usb_put_function is called before
> release_card_device, add refcounting to f_midi_free and
> have rawmidi's private free call it. The f_midi memory
> is only kfreed when usb_put_function and release_card_device
> have both been called.
>
> Signed-off-by: Jerry Zhang <zhangje...@google.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c
> b/drivers/usb/gadget/function/f_midi.c
> index a5719f271bf0..cf52ee2b7e93 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -98,6 +98,7 @@ struct f_midi {
> DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
> spinlock_t transmit_lock;
> unsigned int in_last_port;
> + unsigned char free_ref;
>
> struct gmidi_in_port in_ports_array[/* in_ports */];
> };
> @@ -818,6 +819,8 @@ static int f_midi_register_card(struct f_midi *midi)
> SNDRV_RAWMIDI_INFO_INPUT |
> SNDRV_RAWMIDI_INFO_DUPLEX;
> rmidi->private_data = midi;
> + rmidi->private_free = f_midi_rmidi_free;
> + midi->free_ref++;
>
> /*
> * Yes, rawmidi OUTPUT = USB IN, and rawmidi INPUT = USB OUT.
> @@ -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.
> - 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);
> +}
> +
> static void f_midi_unbind(struct usb_configuration *c, struct usb_function
> *f)
> {
> struct usb_composite_dev *cdev = f->config->cdev;
> @@ -1219,7 +1229,7 @@ static void f_midi_unbind(struct usb_configuration *c,
> struct usb_function *f)
> card = midi->card;
> midi->card = NULL;
> if (card)
> - snd_card_free(card);
> + snd_card_free_when_closed(card);
>
> usb_free_all_descriptors(f);
> }
> @@ -1263,6 +1273,7 @@ static struct usb_function *f_midi_alloc(struct
> usb_function_instance *fi)
> midi->buflen = opts->buflen;
> midi->qlen = opts->qlen;
> midi->in_last_port = 0;
> + midi->free_ref = 1;
>
> status = kfifo_alloc(&midi->in_req_fifo, midi->qlen, GFP_KERNEL);
> if (status)
--
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
