Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
Em Tue, 22 Mar 2016 11:29:34 -0600 Shuah Khan escreveu: > On 03/22/2016 07:03 AM, Shuah Khan wrote: > > On 03/21/2016 10:01 PM, Shuah Khan wrote: > >> On 03/19/2016 07:31 AM, Shuah Khan wrote: > >>> On 03/19/2016 06:10 AM, Mauro Carvalho Chehab wrote: > Em Fri, 18 Mar 2016 20:50:31 -0600 > Shuah Khan 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 > > --- > > > > - 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 > > Yeah, a way better! > > For normal bind/unbind, it seems to be working fine. Also > for driver's rmmod, so: > > Tested-by: Mauro Carvalho Chehab > > Takashi, > > Could please ack this patch - please see below that the problem > Mauro and I both saw ended up to a latent bug in au0828 that is > in Linux 4.5 as well. It is now fixed. FYI, the patches we're intending to send to fix the issues with au0828 and snd-usb-audio are at my experimental tree: https://git.linuxtv.org/mchehab/experimental.git/log/?h=au0828-unbind-fixes The patches are: f9dca0c46f12 [media] au0828: Fix dev_state handling d9898e2e7bb3 [media] au0828: fix au0828_v4l2_close() dev_state race condition 52a6e1f97587 [media] media-devnode: add missing mutex lock in error handler db268d4f59c5 [media] media-device: Simplify compat32 logic 105817f85b02 sound/usb: fix to release stream resources from media_snd_device_delete() 70fafd948468 sound/usb: Fix memory leak in media_snd_stream_delete() during unbind a78a4b10ecd3 sound/usb/media: use core routine to initialize media_device 9d8830150475 [media] media-device: use kref for media_device instance 544439bf084a [media] media-device: make topology_version u64 4e18ca9ce0c2 [media] media-device: Fix a comment c38077d39c7e [media] media-device: get rid of the spinlock a50d06389fdf sound/usb: fix NULL dereference in usb_audio_probe() b39950960d2b [media] media: au0828 fix to clear enable/disable/change source handlers d9f03ad36a9d [media] v4l2-mc: cleanup a warning 6a4f10cff976 [media] au0828: disable tuner links and cache tuner/decoder We're running some stress tests today, so we may need to send a few other patches later on, but I guess they'll be either at au0828 or at the media core. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
On 03/22/2016 07:03 AM, Shuah Khan wrote: > On 03/21/2016 10:01 PM, Shuah Khan wrote: >> On 03/19/2016 07:31 AM, Shuah Khan wrote: >>> On 03/19/2016 06:10 AM, Mauro Carvalho Chehab wrote: Em Fri, 18 Mar 2016 20:50:31 -0600 Shuah Khan 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 > --- > > - 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 Yeah, a way better! For normal bind/unbind, it seems to be working fine. Also for driver's rmmod, so: Tested-by: Mauro Carvalho Chehab Takashi, Could please ack this patch - please see below that the problem Mauro and I both saw ended up to a latent bug in au0828 that is in Linux 4.5 as well. It is now fixed. thanks, -- Shuah PS.: === There are still some troubles if we run an infinite loop binding/unbinding au0828 and another one binding/unbinding snd-usb-audio, like this one: >>> >>> Yes. I noticed this one when I was running a loop of 1000 on au0828. >>> I couldn't reproduce this when I tested this patch. >>> >>> P.S: au08282 loops takes longer btw since au0828 initialization is lot more >>> complex. We have to look at this one. >>> [ 91.556188] [ cut here ] [ 91.556500] WARNING: CPU: 1 PID: 2920 at drivers/media/media-entity.c:392 __media_entity_pipeline_start+0x271/0xce0 [media]() [ 91.556626] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 tuner au8522_decoder au8522_common au0828 videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core tveeprom dvb_core rc_core v4l2_common videodev snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device media cpufreq_powersave cpufreq_conservative cpufreq_userspace cpufreq_stats parport_pc ppdev lp parport snd_hda_codec_hdmi intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel iTCO_wdt kvm iTCO_vendor_support irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha256_ssse3 sha256_generic hmac drbg snd_hda_codec_realtek i915 snd_hda_codec_generic aesni_intel aes_x86_64 btusb lrw btrtl gf128mul snd_hda_intel glue_helper ablk_helper btbcm btintel cryptd psmouse snd_hda_codec bluetooth [ 91.556693] snd_hwdep i2c_algo_bit sg snd_hda_core serio_raw pcspkr evdev drm_kms_helper snd_pcm rfkill drm snd_timer mei_me snd i2c_i801 soundcore lpc_ich mei mfd_core battery video dw_dmac i2c_designware_platform dw_dmac_core i2c_designware_core acpi_pad button tpm_tis tpm ext4 crc16 mbcache jbd2 dm_mod sd_mod hid_generic usbhid ahci libahci libata e1000e xhci_pci ptp scsi_mod ehci_pci ehci_hcd pps_core xhci_hcd fan thermal sdhci_acpi sdhci mmc_core i2c_hid hid [ 91.556748] CPU: 1 PID: 2920 Comm: v4l_id Tainted: G D W 4.5.0+ #62 [ 91.556751] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0350.2015.0812.1722 08/12/2015 [ 91.556754] a0e247a0 8803a3d17b08 819447c1 [ 91.556759] 88009bbe17c0 8803a3d17b48 8114fd16 a0e20651 [ 91.556763] 8803a47c9c58 8803a477d2c8 8803a5ac96f8 dc00 [ 91.556768] Call Trace: [ 91.556774] [] dump_stack+0x85/0xc4 [ 91.556778] [] warn_slowpath_common+0xc6/0x120 [ 91.556783] [] ? __media_entity_pipeline_start+0x271/0xce0 [media] [ 91.556786] [] warn_slowpath_null+0x1a/0x20 [ 91.556790] [] __media_entity_pipeline_start+0x271/0xce0 [media] [ 91.556794] [] ? __schedule+0x78a/0x2570 [ 91.556797] [] ? memset+0x28/0x30 [ 91.556802] [] ? media_entity_pipeline_stop+0x60/0x60 [media] [ 91.556806] [] ? trace_hardirqs_on+0xd/0x10 [ 91.556810] [] ? au0828_enable_source+0x55/0x9f0 [au0828] [ 91.556813] [] ? mutex_trylock+0x400/0x400 [ 91.556818] [] ? au0828_v4l2_close+0xb3/0x590 [au0828] [ 91.556822] [] au0828_enable_source+0x3f4/0x9f0 [au0828] [ 91.556824] [] ? mutex_trylock+0x400/0x400 [ 91.556834] [] v4l_enable_media_source+0x66/0x90 [videodev] [ 91.556839] [] au0828_
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
On 03/21/2016 10:01 PM, Shuah Khan wrote: > On 03/19/2016 07:31 AM, Shuah Khan wrote: >> On 03/19/2016 06:10 AM, Mauro Carvalho Chehab wrote: >>> Em Fri, 18 Mar 2016 20:50:31 -0600 >>> Shuah Khan 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 --- - 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 >>> >>> Yeah, a way better! >>> >>> For normal bind/unbind, it seems to be working fine. Also >>> for driver's rmmod, so: >>> >>> Tested-by: Mauro Carvalho Chehab >>> >>> PS.: >>> === >>> >>> There are still some troubles if we run an infinite loop >>> binding/unbinding au0828 and another one binding/unbinding >>> snd-usb-audio, like this one: >> >> Yes. I noticed this one when I was running a loop of 1000 on au0828. >> I couldn't reproduce this when I tested this patch. >> >> P.S: au08282 loops takes longer btw since au0828 initialization is lot more >> complex. We have to look at this one. >> >>> >>> >>> [ 91.556188] [ cut here ] >>> [ 91.556500] WARNING: CPU: 1 PID: 2920 at >>> drivers/media/media-entity.c:392 __media_entity_pipeline_start+0x271/0xce0 >>> [media]() >>> [ 91.556626] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 >>> tuner au8522_decoder au8522_common au0828 videobuf2_vmalloc >>> videobuf2_memops videobuf2_v4l2 videobuf2_core tveeprom dvb_core rc_core >>> v4l2_common videodev snd_usb_audio snd_usbmidi_lib snd_rawmidi >>> snd_seq_device media cpufreq_powersave cpufreq_conservative >>> cpufreq_userspace cpufreq_stats parport_pc ppdev lp parport >>> snd_hda_codec_hdmi intel_rapl x86_pkg_temp_thermal intel_powerclamp >>> coretemp kvm_intel iTCO_wdt kvm iTCO_vendor_support irqbypass >>> crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha256_ssse3 >>> sha256_generic hmac drbg snd_hda_codec_realtek i915 snd_hda_codec_generic >>> aesni_intel aes_x86_64 btusb lrw btrtl gf128mul snd_hda_intel glue_helper >>> ablk_helper btbcm btintel cryptd psmouse snd_hda_codec bluetooth >>> [ 91.556693] snd_hwdep i2c_algo_bit sg snd_hda_core serio_raw pcspkr >>> evdev drm_kms_helper snd_pcm rfkill drm snd_timer mei_me snd i2c_i801 >>> soundcore lpc_ich mei mfd_core battery video dw_dmac >>> i2c_designware_platform dw_dmac_core i2c_designware_core acpi_pad button >>> tpm_tis tpm ext4 crc16 mbcache jbd2 dm_mod sd_mod hid_generic usbhid ahci >>> libahci libata e1000e xhci_pci ptp scsi_mod ehci_pci ehci_hcd pps_core >>> xhci_hcd fan thermal sdhci_acpi sdhci mmc_core i2c_hid hid >>> [ 91.556748] CPU: 1 PID: 2920 Comm: v4l_id Tainted: G D W >>> 4.5.0+ #62 >>> [ 91.556751] Hardware name: /NUC5i7RYB, BIOS >>> RYBDWi35.86A.0350.2015.0812.1722 08/12/2015 >>> [ 91.556754] a0e247a0 8803a3d17b08 819447c1 >>> >>> [ 91.556759] 88009bbe17c0 8803a3d17b48 8114fd16 >>> a0e20651 >>> [ 91.556763] 8803a47c9c58 8803a477d2c8 8803a5ac96f8 >>> dc00 >>> [ 91.556768] Call Trace: >>> [ 91.556774] [] dump_stack+0x85/0xc4 >>> [ 91.556778] [] warn_slowpath_common+0xc6/0x120 >>> [ 91.556783] [] ? >>> __media_entity_pipeline_start+0x271/0xce0 [media] >>> [ 91.556786] [] warn_slowpath_null+0x1a/0x20 >>> [ 91.556790] [] >>> __media_entity_pipeline_start+0x271/0xce0 [media] >>> [ 91.556794] [] ? __schedule+0x78a/0x2570 >>> [ 91.556797] [] ? memset+0x28/0x30 >>> [ 91.556802] [] ? media_entity_pipeline_stop+0x60/0x60 >>> [media] >>> [ 91.556806] [] ? trace_hardirqs_on+0xd/0x10 >>> [ 91.556810] [] ? au0828_enable_source+0x55/0x9f0 >>> [au0828] >>> [ 91.556813] [] ? mutex_trylock+0x400/0x400 >>> [ 91.556818] [] ? au0828_v4l2_close+0xb3/0x590 [au0828] >>> [ 91.556822] [] au0828_enable_source+0x3f4/0x9f0 >>> [au0828] >>> [ 91.556824] [] ? mutex_trylock+0x400/0x400 >>> [ 91.556834] [] v4l_enable_media_source+0x66/0x90 >>> [videodev] >>> [ 91.556839] [] au0828_v4l2_close+0x25a/0x590 [au0828] >>> [ 91.556846] [] v4l2_release+0xf0/0x210 [videodev] >>> [ 91.556849] [] __fput+0x20f/0x6d0 >>> [ 91.556853] [] fput+0xe/0x10 >>> [ 91.556856] [] task_work_run+0x137/0x200 >>> [ 91.556859] [] exit_to_usermode_loop+0x154/0x180 >>> [ 91.556863] [] ? trace_hardirqs_on_caller+0x16/0x590 >>> [ 91.55
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
On 03/19/2016 07:31 AM, Shuah Khan wrote: > On 03/19/2016 06:10 AM, Mauro Carvalho Chehab wrote: >> Em Fri, 18 Mar 2016 20:50:31 -0600 >> Shuah Khan 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 >>> --- >>> >>> - 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 >> >> Yeah, a way better! >> >> For normal bind/unbind, it seems to be working fine. Also >> for driver's rmmod, so: >> >> Tested-by: Mauro Carvalho Chehab >> >> PS.: >> === >> >> There are still some troubles if we run an infinite loop >> binding/unbinding au0828 and another one binding/unbinding >> snd-usb-audio, like this one: > > Yes. I noticed this one when I was running a loop of 1000 on au0828. > I couldn't reproduce this when I tested this patch. > > P.S: au08282 loops takes longer btw since au0828 initialization is lot more > complex. We have to look at this one. > >> >> >> [ 91.556188] [ cut here ] >> [ 91.556500] WARNING: CPU: 1 PID: 2920 at drivers/media/media-entity.c:392 >> __media_entity_pipeline_start+0x271/0xce0 [media]() >> [ 91.556626] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 >> tuner au8522_decoder au8522_common au0828 videobuf2_vmalloc videobuf2_memops >> videobuf2_v4l2 videobuf2_core tveeprom dvb_core rc_core v4l2_common videodev >> snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device media >> cpufreq_powersave cpufreq_conservative cpufreq_userspace cpufreq_stats >> parport_pc ppdev lp parport snd_hda_codec_hdmi intel_rapl >> x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel iTCO_wdt kvm >> iTCO_vendor_support irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel >> ghash_clmulni_intel sha256_ssse3 sha256_generic hmac drbg >> snd_hda_codec_realtek i915 snd_hda_codec_generic aesni_intel aes_x86_64 >> btusb lrw btrtl gf128mul snd_hda_intel glue_helper ablk_helper btbcm btintel >> cryptd psmouse snd_hda_codec bluetooth >> [ 91.556693] snd_hwdep i2c_algo_bit sg snd_hda_core serio_raw pcspkr >> evdev drm_kms_helper snd_pcm rfkill drm snd_timer mei_me snd i2c_i801 >> soundcore lpc_ich mei mfd_core battery video dw_dmac i2c_designware_platform >> dw_dmac_core i2c_designware_core acpi_pad button tpm_tis tpm ext4 crc16 >> mbcache jbd2 dm_mod sd_mod hid_generic usbhid ahci libahci libata e1000e >> xhci_pci ptp scsi_mod ehci_pci ehci_hcd pps_core xhci_hcd fan thermal >> sdhci_acpi sdhci mmc_core i2c_hid hid >> [ 91.556748] CPU: 1 PID: 2920 Comm: v4l_id Tainted: G D W >> 4.5.0+ #62 >> [ 91.556751] Hardware name: /NUC5i7RYB, BIOS >> RYBDWi35.86A.0350.2015.0812.1722 08/12/2015 >> [ 91.556754] a0e247a0 8803a3d17b08 819447c1 >> >> [ 91.556759] 88009bbe17c0 8803a3d17b48 8114fd16 >> a0e20651 >> [ 91.556763] 8803a47c9c58 8803a477d2c8 8803a5ac96f8 >> dc00 >> [ 91.556768] Call Trace: >> [ 91.556774] [] dump_stack+0x85/0xc4 >> [ 91.556778] [] warn_slowpath_common+0xc6/0x120 >> [ 91.556783] [] ? >> __media_entity_pipeline_start+0x271/0xce0 [media] >> [ 91.556786] [] warn_slowpath_null+0x1a/0x20 >> [ 91.556790] [] >> __media_entity_pipeline_start+0x271/0xce0 [media] >> [ 91.556794] [] ? __schedule+0x78a/0x2570 >> [ 91.556797] [] ? memset+0x28/0x30 >> [ 91.556802] [] ? media_entity_pipeline_stop+0x60/0x60 >> [media] >> [ 91.556806] [] ? trace_hardirqs_on+0xd/0x10 >> [ 91.556810] [] ? au0828_enable_source+0x55/0x9f0 >> [au0828] >> [ 91.556813] [] ? mutex_trylock+0x400/0x400 >> [ 91.556818] [] ? au0828_v4l2_close+0xb3/0x590 [au0828] >> [ 91.556822] [] au0828_enable_source+0x3f4/0x9f0 >> [au0828] >> [ 91.556824] [] ? mutex_trylock+0x400/0x400 >> [ 91.556834] [] v4l_enable_media_source+0x66/0x90 >> [videodev] >> [ 91.556839] [] au0828_v4l2_close+0x25a/0x590 [au0828] >> [ 91.556846] [] v4l2_release+0xf0/0x210 [videodev] >> [ 91.556849] [] __fput+0x20f/0x6d0 >> [ 91.556853] [] fput+0xe/0x10 >> [ 91.556856] [] task_work_run+0x137/0x200 >> [ 91.556859] [] exit_to_usermode_loop+0x154/0x180 >> [ 91.556863] [] ? trace_hardirqs_on_caller+0x16/0x590 >> [ 91.556866] [] syscall_return_slowpath+0x186/0x1c0 >> [ 91.556869] [] entry_SYSCALL_64_fastpath+0xbf/0xc1 >> [ 91.556872] ---[ end trace 3e95e11ff0b9ef
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
On 03/19/2016 06:10 AM, Mauro Carvalho Chehab wrote: > Em Fri, 18 Mar 2016 20:50:31 -0600 > Shuah Khan 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 >> --- >> >> - 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 > > Yeah, a way better! > > For normal bind/unbind, it seems to be working fine. Also > for driver's rmmod, so: > > Tested-by: Mauro Carvalho Chehab > > PS.: > === > > There are still some troubles if we run an infinite loop > binding/unbinding au0828 and another one binding/unbinding > snd-usb-audio, like this one: Yes. I noticed this one when I was running a loop of 1000 on au0828. I couldn't reproduce this when I tested this patch. P.S: au08282 loops takes longer btw since au0828 initialization is lot more complex. We have to look at this one. > > > [ 91.556188] [ cut here ] > [ 91.556500] WARNING: CPU: 1 PID: 2920 at drivers/media/media-entity.c:392 > __media_entity_pipeline_start+0x271/0xce0 [media]() > [ 91.556626] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 > tuner au8522_decoder au8522_common au0828 videobuf2_vmalloc videobuf2_memops > videobuf2_v4l2 videobuf2_core tveeprom dvb_core rc_core v4l2_common videodev > snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device media > cpufreq_powersave cpufreq_conservative cpufreq_userspace cpufreq_stats > parport_pc ppdev lp parport snd_hda_codec_hdmi intel_rapl > x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel iTCO_wdt kvm > iTCO_vendor_support irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel > ghash_clmulni_intel sha256_ssse3 sha256_generic hmac drbg > snd_hda_codec_realtek i915 snd_hda_codec_generic aesni_intel aes_x86_64 btusb > lrw btrtl gf128mul snd_hda_intel glue_helper ablk_helper btbcm btintel cryptd > psmouse snd_hda_codec bluetooth > [ 91.556693] snd_hwdep i2c_algo_bit sg snd_hda_core serio_raw pcspkr evdev > drm_kms_helper snd_pcm rfkill drm snd_timer mei_me snd i2c_i801 soundcore > lpc_ich mei mfd_core battery video dw_dmac i2c_designware_platform > dw_dmac_core i2c_designware_core acpi_pad button tpm_tis tpm ext4 crc16 > mbcache jbd2 dm_mod sd_mod hid_generic usbhid ahci libahci libata e1000e > xhci_pci ptp scsi_mod ehci_pci ehci_hcd pps_core xhci_hcd fan thermal > sdhci_acpi sdhci mmc_core i2c_hid hid > [ 91.556748] CPU: 1 PID: 2920 Comm: v4l_id Tainted: G D W 4.5.0+ > #62 > [ 91.556751] Hardware name: /NUC5i7RYB, BIOS > RYBDWi35.86A.0350.2015.0812.1722 08/12/2015 > [ 91.556754] a0e247a0 8803a3d17b08 819447c1 > > [ 91.556759] 88009bbe17c0 8803a3d17b48 8114fd16 > a0e20651 > [ 91.556763] 8803a47c9c58 8803a477d2c8 8803a5ac96f8 > dc00 > [ 91.556768] Call Trace: > [ 91.556774] [] dump_stack+0x85/0xc4 > [ 91.556778] [] warn_slowpath_common+0xc6/0x120 > [ 91.556783] [] ? > __media_entity_pipeline_start+0x271/0xce0 [media] > [ 91.556786] [] warn_slowpath_null+0x1a/0x20 > [ 91.556790] [] > __media_entity_pipeline_start+0x271/0xce0 [media] > [ 91.556794] [] ? __schedule+0x78a/0x2570 > [ 91.556797] [] ? memset+0x28/0x30 > [ 91.556802] [] ? media_entity_pipeline_stop+0x60/0x60 > [media] > [ 91.556806] [] ? trace_hardirqs_on+0xd/0x10 > [ 91.556810] [] ? au0828_enable_source+0x55/0x9f0 > [au0828] > [ 91.556813] [] ? mutex_trylock+0x400/0x400 > [ 91.556818] [] ? au0828_v4l2_close+0xb3/0x590 [au0828] > [ 91.556822] [] au0828_enable_source+0x3f4/0x9f0 [au0828] > [ 91.556824] [] ? mutex_trylock+0x400/0x400 > [ 91.556834] [] v4l_enable_media_source+0x66/0x90 > [videodev] > [ 91.556839] [] au0828_v4l2_close+0x25a/0x590 [au0828] > [ 91.556846] [] v4l2_release+0xf0/0x210 [videodev] > [ 91.556849] [] __fput+0x20f/0x6d0 > [ 91.556853] [] fput+0xe/0x10 > [ 91.556856] [] task_work_run+0x137/0x200 > [ 91.556859] [] exit_to_usermode_loop+0x154/0x180 > [ 91.556863] [] ? trace_hardirqs_on_caller+0x16/0x590 > [ 91.556866] [] syscall_return_slowpath+0x186/0x1c0 > [ 91.556869] [] entry_SYSCALL_64_fastpath+0xbf/0xc1 > [ 91.556872] ---[ end trace 3e95e11ff0b9efad ]--- > > I suspect that something is not properly protected by the graph mutex at > either ALSA or au0828 driver. > > I also got a GPF: > > [ 95
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
On 03/19/2016 04:39 AM, Mauro Carvalho Chehab wrote:
> Em Fri, 18 Mar 2016 23:57:08 -0300
> Mauro Carvalho Chehab escreveu:
>
>> Em Fri, 18 Mar 2016 20:50:31 -0600
>> Shuah Khan 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
>>> ---
>>>
>>> - 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)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
Em Fri, 18 Mar 2016 20:50:31 -0600 Shuah Khan 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 > --- > > - 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 Yeah, a way better! For normal bind/unbind, it seems to be working fine. Also for driver's rmmod, so: Tested-by: Mauro Carvalho Chehab PS.: === There are still some troubles if we run an infinite loop binding/unbinding au0828 and another one binding/unbinding snd-usb-audio, like this one: [ 91.556188] [ cut here ] [ 91.556500] WARNING: CPU: 1 PID: 2920 at drivers/media/media-entity.c:392 __media_entity_pipeline_start+0x271/0xce0 [media]() [ 91.556626] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 tuner au8522_decoder au8522_common au0828 videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core tveeprom dvb_core rc_core v4l2_common videodev snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device media cpufreq_powersave cpufreq_conservative cpufreq_userspace cpufreq_stats parport_pc ppdev lp parport snd_hda_codec_hdmi intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel iTCO_wdt kvm iTCO_vendor_support irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha256_ssse3 sha256_generic hmac drbg snd_hda_codec_realtek i915 snd_hda_codec_generic aesni_intel aes_x86_64 btusb lrw btrtl gf128mul snd_hda_intel glue_helper ablk_helper btbcm btintel cryptd psmouse snd_hda_codec bluetooth [ 91.556693] snd_hwdep i2c_algo_bit sg snd_hda_core serio_raw pcspkr evdev drm_kms_helper snd_pcm rfkill drm snd_timer mei_me snd i2c_i801 soundcore lpc_ich mei mfd_core battery video dw_dmac i2c_designware_platform dw_dmac_core i2c_designware_core acpi_pad button tpm_tis tpm ext4 crc16 mbcache jbd2 dm_mod sd_mod hid_generic usbhid ahci libahci libata e1000e xhci_pci ptp scsi_mod ehci_pci ehci_hcd pps_core xhci_hcd fan thermal sdhci_acpi sdhci mmc_core i2c_hid hid [ 91.556748] CPU: 1 PID: 2920 Comm: v4l_id Tainted: G D W 4.5.0+ #62 [ 91.556751] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0350.2015.0812.1722 08/12/2015 [ 91.556754] a0e247a0 8803a3d17b08 819447c1 [ 91.556759] 88009bbe17c0 8803a3d17b48 8114fd16 a0e20651 [ 91.556763] 8803a47c9c58 8803a477d2c8 8803a5ac96f8 dc00 [ 91.556768] Call Trace: [ 91.556774] [] dump_stack+0x85/0xc4 [ 91.556778] [] warn_slowpath_common+0xc6/0x120 [ 91.556783] [] ? __media_entity_pipeline_start+0x271/0xce0 [media] [ 91.556786] [] warn_slowpath_null+0x1a/0x20 [ 91.556790] [] __media_entity_pipeline_start+0x271/0xce0 [media] [ 91.556794] [] ? __schedule+0x78a/0x2570 [ 91.556797] [] ? memset+0x28/0x30 [ 91.556802] [] ? media_entity_pipeline_stop+0x60/0x60 [media] [ 91.556806] [] ? trace_hardirqs_on+0xd/0x10 [ 91.556810] [] ? au0828_enable_source+0x55/0x9f0 [au0828] [ 91.556813] [] ? mutex_trylock+0x400/0x400 [ 91.556818] [] ? au0828_v4l2_close+0xb3/0x590 [au0828] [ 91.556822] [] au0828_enable_source+0x3f4/0x9f0 [au0828] [ 91.556824] [] ? mutex_trylock+0x400/0x400 [ 91.556834] [] v4l_enable_media_source+0x66/0x90 [videodev] [ 91.556839] [] au0828_v4l2_close+0x25a/0x590 [au0828] [ 91.556846] [] v4l2_release+0xf0/0x210 [videodev] [ 91.556849] [] __fput+0x20f/0x6d0 [ 91.556853] [] fput+0xe/0x10 [ 91.556856] [] task_work_run+0x137/0x200 [ 91.556859] [] exit_to_usermode_loop+0x154/0x180 [ 91.556863] [] ? trace_hardirqs_on_caller+0x16/0x590 [ 91.556866] [] syscall_return_slowpath+0x186/0x1c0 [ 91.556869] [] entry_SYSCALL_64_fastpath+0xbf/0xc1 [ 91.556872] ---[ end trace 3e95e11ff0b9efad ]--- I suspect that something is not properly protected by the graph mutex at either ALSA or au0828 driver. I also got a GPF: [ 95.398931] au0828: Start Pipeline: Xceive XC5000->au0828a vbi Error -16 [ 95.398936] au0828: Deactivate link Error 0 [ 95.398943] tuner 6-0061: Putting tuner to sleep [ 95.398960] kasan: CONFIG_KASAN_INLINE enabled [ 95.398962] kasan: GPF could be caused by NULL-ptr deref or user memory access [ 95.398967] general protection fault: [#2] SMP KASAN [ 95.399020] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 tuner au8522_decoder au8522_common au0828 vide
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
Em Fri, 18 Mar 2016 23:57:08 -0300
Mauro Carvalho Chehab escreveu:
> Em Fri, 18 Mar 2016 20:50:31 -0600
> Shuah Khan 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
> > ---
> >
> > - 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.
Regards,
Mauro
>
> Regards,
> Mauro
>
> > + }
> >
> > media_snd_mixer_delete(chip);
> >
>
>
--
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
Em Fri, 18 Mar 2016 20:50:31 -0600
Shuah Khan 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
> ---
>
> - 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?
Regards,
Mauro
> + }
>
> media_snd_mixer_delete(chip);
>
--
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()
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
---
- 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]);
+ }
media_snd_mixer_delete(chip);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
