| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20151215091342.2f825d91@recife.lan> Date: Tue, 15 Dec 2015 09:13:42 -0200 From: Mauro Carvalho Chehab <mchehab@....samsung.com> To: Sakari Ailus <sakari.ailus@...ux.intel.com> Cc: Javier Martinez Canillas <javier@....samsung.com>, linux-kernel@...r.kernel.org, Luis de Bethencourt <luis@...ethencourt.com>, linux-sh@...r.kernel.org, Sylwester Nawrocki <s.nawrocki@...sung.com>, Sören Brinkmann <soren.brinkmann@...inx.com>, linux-samsung-soc@...r.kernel.org, Hyun Kwon <hyun.kwon@...inx.com>, Matthias Schwarzott <zzam@...too.org>, Kyungmin Park <kyungmin.park@...sung.com>, Tommi Rantala <tt.rantala@...il.com>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, Hans Verkuil <hans.verkuil@...co.com>, linux-media@...r.kernel.org, Kukjin Kim <kgene@...nel.org>, Krzysztof Kozlowski <k.kozlowski@...sung.com>, Michal Simek <michal.simek@...inx.com>, Olli Salonen <olli.salonen@....fi>, linux-arm-kernel@...ts.infradead.org, Stefan Richter <stefanr@...6.in-berlin.de>, Antti Palosaari <crope@....fi>, Shuah Khan <shuahkh@....samsung.com>, Rafael Lourenço de Lima Chehab <chehabrafael@...il.com> Subject: Re: [PATCH 2/2] [media] media-device: split media initialization and registration Em Thu, 10 Sep 2015 20:14:04 +0300 Sakari Ailus <sakari.ailus@...ux.intel.com> escreveu: > Hi Javier, > > Thanks for the set! A few comments below. > > Javier Martinez Canillas wrote: > > The media device node is registered and so made visible to user-space > > before entities are registered and links created which means that the > > media graph obtained by user-space could be only partially enumerated > > if that happens too early before all the graph has been created. > > > > To avoid this race condition, split the media init and registration > > in separate functions and only register the media device node when > > all the pending subdevices have been registered, either explicitly > > by the driver or asynchronously using v4l2_async_register_subdev(). > > > > Also, add a media_entity_cleanup() function that will destroy the > > graph_mutex that is initialized in media_entity_init(). > > > > Suggested-by: Sakari Ailus <sakari.ailus@...ux.intel.com> > > Signed-off-by: Javier Martinez Canillas <javier@....samsung.com> > > > > --- > > > > drivers/media/common/siano/smsdvb-main.c | 1 + > > drivers/media/media-device.c | 38 +++++++++++++++++++++++---- > > drivers/media/platform/exynos4-is/media-dev.c | 12 ++++++--- > > drivers/media/platform/omap3isp/isp.c | 11 +++++--- > > drivers/media/platform/s3c-camif/camif-core.c | 13 ++++++--- > > drivers/media/platform/vsp1/vsp1_drv.c | 19 ++++++++++---- > > drivers/media/platform/xilinx/xilinx-vipp.c | 11 +++++--- > > drivers/media/usb/au0828/au0828-core.c | 26 +++++++++++++----- > > drivers/media/usb/cx231xx/cx231xx-cards.c | 22 +++++++++++----- > > drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 11 +++++--- > > drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 13 ++++++--- > > drivers/media/usb/siano/smsusb.c | 14 ++++++++-- > > drivers/media/usb/uvc/uvc_driver.c | 9 +++++-- > > include/media/media-device.h | 2 ++ > > 14 files changed, 156 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/media/common/siano/smsdvb-main.c b/drivers/media/common/siano/smsdvb-main.c > > index ab345490a43a..8a1ea2192439 100644 > > --- a/drivers/media/common/siano/smsdvb-main.c > > +++ b/drivers/media/common/siano/smsdvb-main.c > > @@ -617,6 +617,7 @@ static void smsdvb_media_device_unregister(struct smsdvb_client_t *client) > > if (!coredev->media_dev) > > return; > > media_device_unregister(coredev->media_dev); > > + media_device_cleanup(coredev->media_dev); > > kfree(coredev->media_dev); > > coredev->media_dev = NULL; > > #endif > > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c > > index 745defb34b33..a8beb0b445a6 100644 > > --- a/drivers/media/media-device.c > > +++ b/drivers/media/media-device.c > > @@ -526,7 +526,7 @@ static void media_device_release(struct media_devnode *mdev) > > } > > > > /** > > - * media_device_register - register a media device > > + * media_device_init() - initialize a media device > > * @mdev: The media device > > * > > * The caller is responsible for initializing the media device before > > @@ -534,12 +534,11 @@ static void media_device_release(struct media_devnode *mdev) > > * > > * - dev must point to the parent device > > * - model must be filled with the device model name > > + * > > + * returns zero on success or a negative error code. > > */ > > -int __must_check __media_device_register(struct media_device *mdev, > > - struct module *owner) > > +int __must_check media_device_init(struct media_device *mdev) > > I think I suggested making media_device_init() return void as the only > remaining source of errors would be driver bugs. > > I'd simply replace the WARN_ON() below with BUG(). That sounds like bad idea to me, and it is against the current Kernel policy of using BUG() only when there's no other way, e. g. on event so severe that the Kernel has no other thing to do except to stop running. For sure, this is not the case here. Also, all drivers have already a logic that checks if the device init happened. So, they should already be doing the right thing. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists