On Mon, 24 May 2021 19:36:44 +0200, Hyeonggon Yoo wrote:

line6_init_cap_control doesn't free resources properly when allocations like kmalloc, usb_alloc_urb fails. It can cause memory leak when some allocations succeed, and then an error occurs.

This patch makes line6_init_cap_control to properly free the resources, freeing previously allocated resources when there's an error.

Signed-off-by: Hyeonggon Yoo 42.hyeyoo@gmail.com

sound/usb/line6/driver.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index 9602929b7de9..6991cb855023 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -688,34 +688,52 @@ static int line6_init_cap_control(struct usb_line6 *line6)

/* initialize USB buffers: */ line6->buffer_listen = kmalloc(LINE6_BUFSIZE_LISTEN, GFP_KERNEL);

• if (!line6->buffer_listen)

• return -ENOMEM;
  

• if (!line6->buffer_listen) {

• ret = -ENOMEM;
  

• goto fail_ret;
  

• }

  line6->urb_listen = usb_alloc_urb(0, GFP_KERNEL);

• if (!line6->urb_listen)

• return -ENOMEM;
  

• if (!line6->urb_listen) {

• ret = -ENOMEM;
  

• goto fail1;
  

• }

  if (line6->properties->capabilities & LINE6_CAP_CONTROL_MIDI) { line6->buffer_message = kmalloc(LINE6_MIDI_MESSAGE_MAXLEN, GFP_KERNEL);

• if (!line6->buffer_message)
  

• 	return -ENOMEM;
  

• if (!line6->buffer_message) {
  

• 	ret = -ENOMEM;
  

• 	goto fail2;
  

• }
  
  

  ret = line6_init_midi(line6); if (ret < 0)

• 	return ret;
  

• 	goto fail3;
  

  } else { ret = line6_hwdep_init(line6); if (ret < 0)

• 	return ret;
  

• 	goto fail2;
  

  }

  ret = line6_start_listen(line6); if (ret < 0) { dev_err(line6->ifcdev, "cannot start listening: %d\n", ret);

• return ret;
  

• if (line6->properties->capabilities & LINE6_CAP_CONTROL_MIDI)
  

• 	goto fail3;
  

• else
  

• 	goto fail2;
  

  }

  return 0;

+fail3:

• kfree(line6->buffer_message);

+fail2:

• usb_free_urb(line6->urb_listen);

+fail1:

• kfree(line6->buffer_listen);

+fail_ret:

• return ret;

}

Those resources are freed in the common destructor of the card instance, line6_destruct(), at disconnect callback, so it's redundant to release them here; even worse, since you haven't re-initialize with NULL, this change would lead to double-free.

thanks,

Takashi