On 03/19/2016 04:39 AM, Mauro Carvalho Chehab wrote: > Em Fri, 18 Mar 2016 23:57:08 -0300 > Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> escreveu: > >> Em Fri, 18 Mar 2016 20:50:31 -0600 >> Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> escreveu: >> >>> Fix to release stream resources from media_snd_device_delete() before >>> media device is unregistered. Without this change, stream resource free >>> is attempted after the media device is unregistered which would result >>> in use-after-free errors. >>> >>> Signed-off-by: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> >>> --- >>> >>> - Ran bind/unbind loop (1000 iteration) test on snd-usb-audio >>> while running mc_nextgen_test loop (1000 iterations) in parallel. >>> - Ran bind/unbind and rmmod/modprobe tests on both drivers. Also >>> generated graphs when after bind/unbind, rmmod/modprobe. Graphs >>> look good. >>> - Note: Please apply the following patch to fix memory leak: >>> sound/usb: Fix memory leak in media_snd_stream_delete() during unbind >>> https://lkml.org/lkml/2016/3/16/1050 >>> >>> sound/usb/media.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/sound/usb/media.c b/sound/usb/media.c >>> index de4a815..e35af88 100644 >>> --- a/sound/usb/media.c >>> +++ b/sound/usb/media.c >>> @@ -301,6 +301,13 @@ int media_snd_device_create(struct snd_usb_audio *chip, >>> void media_snd_device_delete(struct snd_usb_audio *chip) >>> { >>> struct media_device *mdev = chip->media_dev; >>> + struct snd_usb_stream *stream; >>> + >>> + /* release resources */ >>> + list_for_each_entry(stream, &chip->pcm_list, list) { >>> + media_snd_stream_delete(&stream->substream[0]); >>> + media_snd_stream_delete(&stream->substream[1]); >> >> I'll look on it better tomorrow, but it sounds weird to hardcode >> substream[0] and [1] here... are you sure that this is valid for >> *all* devices supported by snd-usb-audio? > > After looking at pcm.c and finding this: > > static void snd_usb_audio_stream_free(struct snd_usb_stream *stream) > { > free_substream(&stream->substream[0]); > free_substream(&stream->substream[1]); > list_del(&stream->list); > kfree(stream); > } > > It seems that assuming that substream is always an array with size 2 > is right. > > I'll do some tests with it today with your patch. > Right. snd-usb-audio uses this in several places like the one above you found. thanks, -- Shuah -- Shuah Khan Sr. Linux Kernel Developer Open Source Innovation Group Samsung Research America (Silicon Valley) shuahkh@xxxxxxxxxxxxxxx | (970) 217-8978 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
