[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZjO46Uo_tVcRTdA0@kekkonen.localdomain>
Date: Thu, 2 May 2024 16:01:45 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Julien Massot <julien.massot@...labora.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
Jacopo Mondi <jacopo+renesas@...ndi.org>,
Kieran Bingham <kieran.bingham+renesas@...asonboard.com>,
Niklas Söderlund <niklas.soderlund+renesas@...natech.se>,
Benjamin Mugnier <benjamin.mugnier@...s.st.com>,
Sylvain Petinot <sylvain.petinot@...s.st.com>,
Yong Zhi <yong.zhi@...el.com>, Bingbu Cao <bingbu.cao@...el.com>,
Dan Scally <djrscally@...il.com>,
Tianshu Qiu <tian.shu.qiu@...el.com>,
Eugen Hristev <eugen.hristev@...labora.com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
Maxime Ripard <mripard@...nel.org>,
Rui Miguel Silva <rmfrfs@...il.com>,
Martin Kepplinger <martink@...teo.de>,
Purism Kernel Team <kernel@...i.sm>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, Robert Foss <rfoss@...nel.org>,
Todor Tomov <todor.too@...il.com>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
Dafna Hirschfeld <dafna@...tmail.com>,
Heiko Stuebner <heiko@...ech.de>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Alim Akhtar <alim.akhtar@...sung.com>,
Hugues Fruchet <hugues.fruchet@...s.st.com>,
Alain Volmat <alain.volmat@...s.st.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Samuel Holland <samuel@...lland.org>,
Yong Deng <yong.deng@...ewell.com>,
Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
Benoit Parrot <bparrot@...com>, Jai Luthra <j-luthra@...com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Michal Simek <michal.simek@....com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
Sowjanya Komatineni <skomatineni@...dia.com>,
Luca Ceresoli <luca.ceresoli@...tlin.com>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, imx@...ts.linux.dev,
linux-arm-msm@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
linux-rockchip@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-sunxi@...ts.linux.dev, linux-staging@...ts.linux.dev,
linux-tegra@...r.kernel.org
Subject: Re: [PATCH 0/2] Introduce v4l2_async_nf_unregister_cleanup
Hi Laurent,
On Thu, May 02, 2024 at 06:56:26PM +0300, Laurent Pinchart wrote:
> Hi Julien,
>
> On Thu, May 02, 2024 at 05:22:20PM +0200, Julien Massot wrote:
> > Many drivers has
> > v4l2_async_nf_unregister(¬ifier);
> > v4l2_async_nf_cleanup(¬ifier);
> >
> > Introduce a helper function to call both functions in one line.
>
> Does this really go in the right direction ? For other objects (video
> devices, media devices, ...), the unregistration should be done at
> .remove() time, and the cleanup at .release() time (the operation called
> when the last reference to the object is released). This is needed to
> ensure proper lifetime management of the objects, and avoid a
> use-after-free for objects that can be reached from userspace.
>
> It could be argued that the notifier isn't exposed to userspace, but can
> we guarantee that no driver will have a need to access the notifier in a
> code path triggered by a userspace operation ? I think it would be safer
> to adopt the same split for the nofifier unregistration and cleanup. In
> my opinion using the same rule across different APIs also make it easier
> for driver authors and for reviewers to get it right.
>
> As shown by your series, lots of drivers call v4l2_async_nf_cleanup()
> and .remove() time instead of .release(). That's because most drivers
> get lifetime management wrong and don't even implement .release().
> That's something Sakari is addressing with ongoing work. This patch
> series seems to go in the opposite direction.
This still avoids the driver authors feeling they need to implement wrapper
functions for v4l2_async_nf_{unregister,cleanup}. I'd be in favour merging
this.
I don't see this getting in the way of adding use counts as the code will
need to be changed in any case.
--
Regards,
Sakari Ailus
Powered by blists - more mailing lists