Re: [RFC PATCH v2 0/5] Media Device Allocator API
Em Tue, 5 Apr 2016 08:10:11 +0200 Takashi Iwaiescreveu: > On Tue, 05 Apr 2016 05:35:55 +0200, > Shuah Khan wrote: > > > > There are known problems with media device life time management. When media > > device is released while an media ioctl is in progress, ioctls fail with > > use-after-free errors and kernel hangs in some cases. > > > > Media Device can be in any the following states: > > > > - Allocated > > - Registered (could be tied to more than one driver) > > - Unregistered, not in use (media device file is not open) > > - Unregistered, in use (media device file is not open) > > - Released > > > > When media device belongs to more than one driver, registrations should be > > tracked to avoid unregistering when one of the drivers does unregister. A > > new > > num_drivers field in the struct media_device covers this case. The media > > device > > should be unregistered only when the last unregister occurs with num_drivers > > count zero. > > > > When a media device is in use when it is unregistered, it should not be > > released until the application exits when it detects the unregistered > > status. Media device that is in use when it is unregistered is moved to > > to_delete_list. When the last unregister occurs, media device is > > unregistered > > and becomes an unregistered, still allocated device. Unregister marks the > > device to be deleted. > > > > When media device belongs to more than one driver, as both drivers could be > > unbound/bound, driver should not end up getting stale media device that is > > on its way out. Moving the unregistered media device to to_delete_list helps > > this case as well. > > > > I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while > > running application that does ioctls. Didn't see any use-after-free errors > > on media device. A couple of known issues seen: > > > > 1. When application exits, cdev_put() gets called after media device is > >released. This is a known issue to resolve and Media Device Allocator > >can't solve this one. > > 2. When au0828 module is removed and then ioctls fail when cdev_get() looks > >for the owning module as au0828 is very often the module that owns the > >media devnode. This is a cdev related issue that needs to be resolved and > >Media Device Allocator can't solve this one. > > > > Shuah Khan (5): > > media: Add Media Device Allocator API > > media: Add driver count to keep track of media device registrations > > media: uvcvideo change to use Media Device Allocator API > > media: au0828 change to use Media Device Allocator API > > sound/usb: Use Media Controller API to share media resources > > I don't think we need to include usb-audio patch at this stage yet. I agree. Let's first fix MC races first, then address the multi-driver issues. Only after having those fixed, we should look at the sound/usb patch. Ok, we could keep it on some testing tree, but, IMHO, it doesn't make any sense to submit it to review, while the core is not fixed. > The most important thing for now is to improve / stabilize the API > itself so that other drivers can use it as is. Once when the API is > really stabilized, we create a solid git branch that may be based for > multiple subsystems, and I'll merge usb-audio stuff through sound git > tree. Works for me. After we have this properly fixed and stabilized on media, I'll pass you a stable topic branch for you. This way, you can test a new version of the sound/usb patch and apply on your tree when it fits well for you. > > Also, the previous usb-audio MC implementation had a few serious bugs, > including quirk NULL dereference. See the bugzilla below for some fix > patches to 4.6-rc1: > https://bugzilla.kernel.org/show_bug.cgi?id=115561 > Feel free to fold them in, if they are still valid. > > > thanks, > > Takashi -- Cheers, 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: [RFC PATCH v2 0/5] Media Device Allocator API
On 04/05/2016 12:10 AM, Takashi Iwai wrote: > On Tue, 05 Apr 2016 05:35:55 +0200, > Shuah Khan wrote: >> >> There are known problems with media device life time management. When media >> device is released while an media ioctl is in progress, ioctls fail with >> use-after-free errors and kernel hangs in some cases. >> >> Media Device can be in any the following states: >> >> - Allocated >> - Registered (could be tied to more than one driver) >> - Unregistered, not in use (media device file is not open) >> - Unregistered, in use (media device file is not open) >> - Released >> >> When media device belongs to more than one driver, registrations should be >> tracked to avoid unregistering when one of the drivers does unregister. A new >> num_drivers field in the struct media_device covers this case. The media >> device >> should be unregistered only when the last unregister occurs with num_drivers >> count zero. >> >> When a media device is in use when it is unregistered, it should not be >> released until the application exits when it detects the unregistered >> status. Media device that is in use when it is unregistered is moved to >> to_delete_list. When the last unregister occurs, media device is unregistered >> and becomes an unregistered, still allocated device. Unregister marks the >> device to be deleted. >> >> When media device belongs to more than one driver, as both drivers could be >> unbound/bound, driver should not end up getting stale media device that is >> on its way out. Moving the unregistered media device to to_delete_list helps >> this case as well. >> >> I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while >> running application that does ioctls. Didn't see any use-after-free errors >> on media device. A couple of known issues seen: >> >> 1. When application exits, cdev_put() gets called after media device is >>released. This is a known issue to resolve and Media Device Allocator >>can't solve this one. >> 2. When au0828 module is removed and then ioctls fail when cdev_get() looks >>for the owning module as au0828 is very often the module that owns the >>media devnode. This is a cdev related issue that needs to be resolved and >>Media Device Allocator can't solve this one. >> >> Shuah Khan (5): >> media: Add Media Device Allocator API >> media: Add driver count to keep track of media device registrations >> media: uvcvideo change to use Media Device Allocator API >> media: au0828 change to use Media Device Allocator API >> sound/usb: Use Media Controller API to share media resources > > I don't think we need to include usb-audio patch at this stage yet. > The most important thing for now is to improve / stabilize the API > itself so that other drivers can use it as is. Once when the API is > really stabilized, we create a solid git branch that may be based for > multiple subsystems, and I'll merge usb-audio stuff through sound git > tree. Agreed. I included snd-usb-audio as it provides a good test case for multiple driver use-case. Yes it is a good idea to have a git branch for wider testing. > > Also, the previous usb-audio MC implementation had a few serious bugs, > including quirk NULL dereference. See the bugzilla below for some fix > patches to 4.6-rc1: > https://bugzilla.kernel.org/show_bug.cgi?id=115561 > Feel free to fold them in, if they are still valid. I folded them in. It is unfortunate that these bugs were introduced towards the end when I was making changes to address review comments and I didn't catch them, especially the quirk NULL dereference. thanks, -- Shuah -- 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: [RFC PATCH v2 0/5] Media Device Allocator API
On Tue, 05 Apr 2016 05:35:55 +0200, Shuah Khan wrote: > > There are known problems with media device life time management. When media > device is released while an media ioctl is in progress, ioctls fail with > use-after-free errors and kernel hangs in some cases. > > Media Device can be in any the following states: > > - Allocated > - Registered (could be tied to more than one driver) > - Unregistered, not in use (media device file is not open) > - Unregistered, in use (media device file is not open) > - Released > > When media device belongs to more than one driver, registrations should be > tracked to avoid unregistering when one of the drivers does unregister. A new > num_drivers field in the struct media_device covers this case. The media > device > should be unregistered only when the last unregister occurs with num_drivers > count zero. > > When a media device is in use when it is unregistered, it should not be > released until the application exits when it detects the unregistered > status. Media device that is in use when it is unregistered is moved to > to_delete_list. When the last unregister occurs, media device is unregistered > and becomes an unregistered, still allocated device. Unregister marks the > device to be deleted. > > When media device belongs to more than one driver, as both drivers could be > unbound/bound, driver should not end up getting stale media device that is > on its way out. Moving the unregistered media device to to_delete_list helps > this case as well. > > I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while > running application that does ioctls. Didn't see any use-after-free errors > on media device. A couple of known issues seen: > > 1. When application exits, cdev_put() gets called after media device is >released. This is a known issue to resolve and Media Device Allocator >can't solve this one. > 2. When au0828 module is removed and then ioctls fail when cdev_get() looks >for the owning module as au0828 is very often the module that owns the >media devnode. This is a cdev related issue that needs to be resolved and >Media Device Allocator can't solve this one. > > Shuah Khan (5): > media: Add Media Device Allocator API > media: Add driver count to keep track of media device registrations > media: uvcvideo change to use Media Device Allocator API > media: au0828 change to use Media Device Allocator API > sound/usb: Use Media Controller API to share media resources I don't think we need to include usb-audio patch at this stage yet. The most important thing for now is to improve / stabilize the API itself so that other drivers can use it as is. Once when the API is really stabilized, we create a solid git branch that may be based for multiple subsystems, and I'll merge usb-audio stuff through sound git tree. Also, the previous usb-audio MC implementation had a few serious bugs, including quirk NULL dereference. See the bugzilla below for some fix patches to 4.6-rc1: https://bugzilla.kernel.org/show_bug.cgi?id=115561 Feel free to fold them in, if they are still valid. thanks, Takashi -- 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
[RFC PATCH v2 0/5] Media Device Allocator API
There are known problems with media device life time management. When media device is released while an media ioctl is in progress, ioctls fail with use-after-free errors and kernel hangs in some cases. Media Device can be in any the following states: - Allocated - Registered (could be tied to more than one driver) - Unregistered, not in use (media device file is not open) - Unregistered, in use (media device file is not open) - Released When media device belongs to more than one driver, registrations should be tracked to avoid unregistering when one of the drivers does unregister. A new num_drivers field in the struct media_device covers this case. The media device should be unregistered only when the last unregister occurs with num_drivers count zero. When a media device is in use when it is unregistered, it should not be released until the application exits when it detects the unregistered status. Media device that is in use when it is unregistered is moved to to_delete_list. When the last unregister occurs, media device is unregistered and becomes an unregistered, still allocated device. Unregister marks the device to be deleted. When media device belongs to more than one driver, as both drivers could be unbound/bound, driver should not end up getting stale media device that is on its way out. Moving the unregistered media device to to_delete_list helps this case as well. I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while running application that does ioctls. Didn't see any use-after-free errors on media device. A couple of known issues seen: 1. When application exits, cdev_put() gets called after media device is released. This is a known issue to resolve and Media Device Allocator can't solve this one. 2. When au0828 module is removed and then ioctls fail when cdev_get() looks for the owning module as au0828 is very often the module that owns the media devnode. This is a cdev related issue that needs to be resolved and Media Device Allocator can't solve this one. Shuah Khan (5): media: Add Media Device Allocator API media: Add driver count to keep track of media device registrations media: uvcvideo change to use Media Device Allocator API media: au0828 change to use Media Device Allocator API sound/usb: Use Media Controller API to share media resources drivers/media/Makefile | 3 +- drivers/media/media-dev-allocator.c| 154 drivers/media/media-device.c | 49 - drivers/media/media-devnode.c | 10 +- drivers/media/usb/au0828/au0828-core.c | 40 +++-- drivers/media/usb/au0828/au0828.h | 1 + drivers/media/usb/uvc/uvc_driver.c | 36 ++-- drivers/media/usb/uvc/uvcvideo.h | 3 +- include/media/media-dev-allocator.h| 116 include/media/media-device.h | 31 sound/usb/Kconfig | 4 + sound/usb/Makefile | 2 + sound/usb/card.c | 14 ++ sound/usb/card.h | 3 + sound/usb/media.c | 320 + sound/usb/media.h | 73 sound/usb/mixer.h | 3 + sound/usb/pcm.c| 28 ++- sound/usb/quirks-table.h | 1 + sound/usb/stream.c | 2 + sound/usb/usbaudio.h | 6 + 21 files changed, 862 insertions(+), 37 deletions(-) create mode 100644 drivers/media/media-dev-allocator.c create mode 100644 include/media/media-dev-allocator.h create mode 100644 sound/usb/media.c create mode 100644 sound/usb/media.h -- 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