mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] media: v4l2-core: Fix Use-After-Free in v4l2_subdev_get_fwnode_pad_1_to_1
@ 2026-06-16  9:25 Biren Pandya
  2026-06-16 13:50 ` Laurent Pinchart
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Biren Pandya @ 2026-06-16  9:25 UTC (permalink / raw)
  To: laurent.pinchart, sakari.ailus, hverkuil+cisco
  Cc: mchehab, linux-media, linux-kernel, Biren Pandya

v4l2_subdev_get_fwnode_pad_1_to_1() drops the fwnode reference via
fwnode_handle_put() before passing it to device_match_fwnode(). This
creates a Use-After-Free vulnerability. If the freed memory is instantly
reallocated by SLUB, the pointer comparison could accidentally match
the wrong pad or trigger a KASAN panic.

Fix this by using the __free(fwnode_handle) scoped guard. This safely
binds the fwnode lifecycle to the function scope, holding the reference
during the match and releasing it automatically upon return.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 831c69c958b8..e6b133ef7850 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -7,7 +7,7 @@
  * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
  *	    Sakari Ailus <sakari.ailus@iki.fi>
  */
-
+#include <linux/cleanup.h>
 #include <linux/export.h>
 #include <linux/ioctl.h>
 #include <linux/leds.h>
@@ -1243,7 +1243,7 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
 int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
 				      struct fwnode_endpoint *endpoint)
 {
-	struct fwnode_handle *fwnode;
+	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
 	struct v4l2_subdev *sd;
 
 	if (!is_media_entity_v4l2_subdev(entity))
@@ -1252,7 +1252,6 @@ int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
 	sd = media_entity_to_v4l2_subdev(entity);
 
 	fwnode = fwnode_graph_get_port_parent(endpoint->local_fwnode);
-	fwnode_handle_put(fwnode);
 
 	if (device_match_fwnode(sd->dev, fwnode))
 		return endpoint->port;

-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: v4l2-core: Fix Use-After-Free in v4l2_subdev_get_fwnode_pad_1_to_1
  2026-06-16  9:25 [PATCH] media: v4l2-core: Fix Use-After-Free in v4l2_subdev_get_fwnode_pad_1_to_1 Biren Pandya
@ 2026-06-16 13:50 ` Laurent Pinchart
  2026-06-16 15:43 ` [PATCH v2] media: v4l2-core: Drop manual fwnode_handle_put() via scope-based cleanup Biren Pandya
  2026-06-16 16:22 ` Biren Pandya
  2 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2026-06-16 13:50 UTC (permalink / raw)
  To: Biren Pandya
  Cc: sakari.ailus, hverkuil+cisco, mchehab, linux-media, linux-kernel

On Tue, Jun 16, 2026 at 02:55:16PM +0530, Biren Pandya wrote:
> v4l2_subdev_get_fwnode_pad_1_to_1() drops the fwnode reference via
> fwnode_handle_put() before passing it to device_match_fwnode(). This
> creates a Use-After-Free vulnerability. If the freed memory is instantly
> reallocated by SLUB, the pointer comparison could accidentally match
> the wrong pad or trigger a KASAN panic.

There's no vulnerability in practice. The code change is still worth it
in my opinion, but the commit message needs an update.

> Fix this by using the __free(fwnode_handle) scoped guard. This safely
> binds the fwnode lifecycle to the function scope, holding the reference
> during the match and releasing it automatically upon return.
> 
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 831c69c958b8..e6b133ef7850 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -7,7 +7,7 @@
>   * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>   *	    Sakari Ailus <sakari.ailus@iki.fi>
>   */
> -
> +#include <linux/cleanup.h>
>  #include <linux/export.h>
>  #include <linux/ioctl.h>
>  #include <linux/leds.h>
> @@ -1243,7 +1243,7 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
>  int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
>  				      struct fwnode_endpoint *endpoint)
>  {
> -	struct fwnode_handle *fwnode;
> +	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
>  	struct v4l2_subdev *sd;
>  
>  	if (!is_media_entity_v4l2_subdev(entity))
> @@ -1252,7 +1252,6 @@ int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
>  	sd = media_entity_to_v4l2_subdev(entity);
>  
>  	fwnode = fwnode_graph_get_port_parent(endpoint->local_fwnode);

The recommended usage is to declare the variable where initialized:

	struct fwnode_handle *fwnode __free(fwnode_handle) =
		fwnode_graph_get_port_parent(endpoint->local_fwnode);

> -	fwnode_handle_put(fwnode);
>  
>  	if (device_match_fwnode(sd->dev, fwnode))
>  		return endpoint->port;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] media: v4l2-core: Drop manual fwnode_handle_put() via scope-based cleanup
  2026-06-16  9:25 [PATCH] media: v4l2-core: Fix Use-After-Free in v4l2_subdev_get_fwnode_pad_1_to_1 Biren Pandya
  2026-06-16 13:50 ` Laurent Pinchart
@ 2026-06-16 15:43 ` Biren Pandya
  2026-06-16 16:22 ` Biren Pandya
  2 siblings, 0 replies; 4+ messages in thread
From: Biren Pandya @ 2026-06-16 15:43 UTC (permalink / raw)
  To: sakari.ailus, laurent.pinchart, mchehab
  Cc: linux-media, linux-kernel, Biren Pandya

v4l2_subdev_get_fwnode_pad_1_to_1() acquires a fwnode reference and drops
it via fwnode_handle_put() before calling device_match_fwnode(). This
requires manual lifecycle management.

Simplify the function by converting the local fwnode variable to use the
__free(fwnode_handle) scope-based cleanup macro.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
Changes in v2:
- Updated the commit message to remove the "Use-After-Free" language,
  as there is no vulnerability in practice.
- Adjusted the implementation to declare `struct fwnode_handle *fwnode`
  directly at the point of initialization, rather than at the top of the
  block, conforming to recommended usage.
- Renamed the patch subject to better reflect the true nature of the change.
- Link to v1: https://lore.kernel.org/all/20260616092516.46339-1-birenpandya@gmail.com/

Hi Laurent,

My sincere apologies for the mailing list noise earlier. That duplicate 
submission was an accidental misfire while I was adjusting my `git send-email`
scripts, and I deeply apologize for the lack of versioning and threading. 
I am still getting up to speed with the subsystem's submission workflow! 
Here is the correctly formatted v2 addressing Sakari's feedback.

 drivers/media/v4l2-core/v4l2-subdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index e6b133ef7850..806b059410ce 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1243,7 +1243,8 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
 int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
 				      struct fwnode_endpoint *endpoint)
 {
-	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
+	struct fwnode_handle *fwnode __free(fwnode_handle) =
+		fwnode_graph_get_port_parent(endpoint->local_fwnode);
 	struct v4l2_subdev *sd;
 
 	if (!is_media_entity_v4l2_subdev(entity))
@@ -1251,8 +1252,6 @@ int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
 
 	sd = media_entity_to_v4l2_subdev(entity);
 
-	fwnode = fwnode_graph_get_port_parent(endpoint->local_fwnode);
-
 	if (device_match_fwnode(sd->dev, fwnode))
 		return endpoint->port;
 
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] media: v4l2-core: Drop manual fwnode_handle_put() via scope-based cleanup
  2026-06-16  9:25 [PATCH] media: v4l2-core: Fix Use-After-Free in v4l2_subdev_get_fwnode_pad_1_to_1 Biren Pandya
  2026-06-16 13:50 ` Laurent Pinchart
  2026-06-16 15:43 ` [PATCH v2] media: v4l2-core: Drop manual fwnode_handle_put() via scope-based cleanup Biren Pandya
@ 2026-06-16 16:22 ` Biren Pandya
  2 siblings, 0 replies; 4+ messages in thread
From: Biren Pandya @ 2026-06-16 16:22 UTC (permalink / raw)
  To: sakari.ailus, laurent.pinchart, mchehab, hverkuil+cisco
  Cc: jacopo.mondi, tomi.valkeinen+renesas, linux-media, linux-kernel,
	Biren Pandya

Simplify v4l2_subdev_get_fwnode_pad_1_to_1() by converting the local
fwnode variable to use the __free(fwnode_handle) scope-based cleanup
macro.

This removes the need for manual fwnode_handle_put() calls and naturally
ensures the fwnode reference is held during the device_match_fwnode()
comparison.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
Changes in v2:
- Updated the commit message to remove the "Use-After-Free" language,
  as there is no vulnerability in practice.
- Adjusted the implementation to declare `struct fwnode_handle *fwnode`
  directly at the point of initialization, rather than at the top of the
  block, conforming to recommended usage.
- Renamed the patch subject to better reflect the true nature of the change.
- Link to v1: https://lore.kernel.org/all/20260616092516.46339-1-birenpandya@gmail.com/

 drivers/media/v4l2-core/v4l2-subdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index e6b133ef7850..806b059410ce 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1243,15 +1243,14 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
 int v4l2_subdev_get_fwnode_pad_1_to_1(struct media_entity *entity,
 				      struct fwnode_endpoint *endpoint)
 {
-	struct fwnode_handle *fwnode;
+	struct fwnode_handle *fwnode __free(fwnode_handle) =
+		fwnode_graph_get_port_parent(endpoint->local_fwnode);
 	struct v4l2_subdev *sd;
 
 	if (!is_media_entity_v4l2_subdev(entity))
 		return -EINVAL;
 
 	sd = media_entity_to_v4l2_subdev(entity);
 
-	fwnode = fwnode_graph_get_port_parent(endpoint->local_fwnode);
-	fwnode_handle_put(fwnode);
 
 	if (device_match_fwnode(sd->dev, fwnode))
 		return endpoint->port;
 
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-16 16:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-16  9:25 [PATCH] media: v4l2-core: Fix Use-After-Free in v4l2_subdev_get_fwnode_pad_1_to_1 Biren Pandya
2026-06-16 13:50 ` Laurent Pinchart
2026-06-16 15:43 ` [PATCH v2] media: v4l2-core: Drop manual fwnode_handle_put() via scope-based cleanup Biren Pandya
2026-06-16 16:22 ` Biren Pandya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®