Hi Laurent,
-----Original Message----- From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 19 November 2024 16:28 Subject: Re: [PATCH v5 1/3] drm: adv7511: Fix use-after-free in adv7533_attach_dsi()
Hi Biju,
Thank you for the patch.
On Tue, Nov 19, 2024 at 01:10:03PM +0000, Biju Das wrote:
The host_node pointer was assigned and freed in adv7533_parse_dt(), and later, adv7533_attach_dsi() uses the same. Fix this use-after-free issue by dropping of_node_put() in adv7533_parse_dt() and calling of_node_put() in error path of probe() and also in the remove().
Fixes: 1e4d58cd7f88 ("drm/bridge: adv7533: Create a MIPI DSI device") Cc: stable@vger.kernel.org Signed-off-by: Biju Das biju.das.jz@bp.renesas.com
Changes in v5:
- Updated commit description.
- restored host_node in struct adv7511.
- Dropped of_node_put() in adv7533_parse_dt() and calling of_node_put() in error path of probe() and also in the remove().
Changes in v4:
- Updated commit description.
- Dropped host_node from struct adv7511 and instead used a local pointer in probe(). Also freeing of host_node pointer after use is done in probe().
Changes in v3:
- Replace __free construct with readable of_node_put().
Changes in v2:
- Added the tag "Cc: stable@vger.kernel.org" in the sign-off area.
- Dropped Archit Taneja invalid Mail address
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 +++++ drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index eb5919b38263..6cfdda04f52f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -1363,6 +1363,8 @@ static int adv7511_probe(struct i2c_client *i2c) i2c_unregister_device(adv7511->i2c_edid); uninit_regulators: adv7511_uninit_regulators(adv7511);
- if (adv7511->host_node)
of_node_put(adv7511->host_node);This won't be called when adv7511_init_regulators() fails as the driver returns directly then, leaking the reference. You need a new error label and a goto for that error path.
Oops missed again.
In the future, when touching error handling, please try to check existing error paths and verify they're still right.
Yes, will take care next time.
Cheers, Biju
return ret; } @@ -1371,6 +1373,9 @@ static void adv7511_remove(struct i2c_client *i2c) { struct adv7511 *adv7511 = i2c_get_clientdata(i2c);
if (adv7511->host_node)
of_node_put(adv7511->host_node);adv7511_uninit_regulators(adv7511);
drm_bridge_remove(&adv7511->bridge);
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index 4481489aaf5e..5f195e91b3e6 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -181,8 +181,6 @@ int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) if (!adv->host_node) return -ENODEV;
- of_node_put(adv->host_node);
- adv->use_timing_gen = !of_property_read_bool(np, "adi,disable-timing-generator");
-- Regards,
Laurent Pinchart