Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Race condition and Use-After-Free due to unreferenced pointer returned from drm_dp_aux_find_by_of_node(). - [High] Ignored get_device() failure leaves a dangling pointer leading to Use-After-Free on error paths. - [High] Unbalanced i2c_put_adapter() on error path causes I2C adapter reference count underflow. -- commit 214c0c58066e5c28266a88ecb7dc4e05a84c1ad9 Author: Christophe JAILLET <[email protected]> drm/tegra: Fix a strange error handling path The commit fixes an error handling path in tegra_sor_probe() by returning directly when devm_tegra_pmc_get() fails, instead of jumping to the put_aux label which would release an uninitialized resource. > diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c > index 4032c6ad45bc1..e384cbd0cbf72 100644 > --- a/drivers/gpu/drm/tegra/sor.c > +++ b/drivers/gpu/drm/tegra/sor.c > @@ -3764,10 +3764,8 @@ static int tegra_sor_probe(struct platform_device > *pdev) > sor->num_settings = sor->soc->num_settings; > > sor->pmc = devm_tegra_pmc_get(&pdev->dev); > - if (IS_ERR(sor->pmc)) { > - err = PTR_ERR(sor->pmc); > - goto put_aux; > - } > + if (IS_ERR(sor->pmc)) > + return PTR_ERR(sor->pmc); > > np = of_parse_phandle(pdev->dev.of_node, "nvidia,dpaux", 0); > if (np) { [Severity: High] This is a pre-existing issue, but can this lead to a race condition and a use-after-free? Looking at drm_dp_aux_find_by_of_node() in drivers/gpu/drm/tegra/dpaux.c, it searches dpaux_list while holding dpaux_lock. Upon finding a match, it drops the lock and returns a pointer to dpaux->aux without taking a reference count on the device. Because tegra_dpaux is a separate platform device, it can be concurrently unbound, triggering tegra_dpaux_remove() and freeing the memory via devres. When tegra_sor_probe() receives this pointer and calls get_device(sor->aux->dev), if tegra_dpaux_remove() executes after the lock is released but before get_device() is called, will this dereference freed memory? [Severity: High] This isn't a bug introduced by this patch, but does the failure path of get_device() leave a dangling pointer? In tegra_sor_probe(): if (get_device(sor->aux->dev)) sor->output.ddc = &sor->aux->ddc; If get_device() fails (returns NULL if the aux device is concurrently removed), the failure is silently ignored. The output.ddc assignment is skipped, but sor->aux remains set to the invalid pointer. If a subsequent initialization step fails, the probe function jumps to the put_aux label: put_aux: if (sor->aux) put_device(sor->aux->dev); Will this read the device structure from the dangling pointer and call put_device() on freed memory, leading to a use-after-free? [Severity: High] This is a pre-existing issue, but does the error path in tegra_output_probe() cause an I2C adapter reference count underflow? In tegra_sor_probe(), sor->output.ddc is aliased to &sor->aux->ddc without explicitly taking an I2C adapter reference via i2c_get_adapter(). The code then calls tegra_output_probe(&sor->output). If this function encounters an error, it jumps to the put_i2c label: drivers/gpu/drm/tegra/output.c:tegra_output_probe() { ... put_i2c: if (output->ddc) i2c_put_adapter(output->ddc); ... } Since output->ddc points to the alias, i2c_put_adapter() will drop the device and module reference without a matching get. Can this cause the adapter to be prematurely freed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/308ea09281bd0b3fb0ec7e90e23012c0783f373c.1781983379.git.christophe.jail...@wanadoo.fr?part=1
