linux-kernel - [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20171115123823.1895515-1-arnd@arndb.de>
Date:   Wed, 15 Nov 2017 13:37:57 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Archit Taneja <architt@...eaurora.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Xinliang Liu <xinliang.liu@...aro.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Sean Paul <seanpaul@...omium.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        David Airlie <airlied@...ux.ie>,
        John Stultz <john.stultz@...aro.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Bhumika Goyal <bhumirks@...il.com>,
        Inki Dae <inki.dae@...sung.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec_init() failure handling

An otherwise correct cleanup patch from Dan Carpenter turned a broken
failure handling from a feature patch by Hans Verkuil into a kernel
Oops, so bisection points to commit 7af35b0addbc ("drm/kirin: Checking
for IS_ERR() instead of NULL") rather than 3b1b975003e4 ("drm:
adv7511/33: add HDMI CEC support").

I've managed to piece together several partial problems, though
I'm still struggling with the bigger picture:

adv7511_probe() registers a drm_bridge structure that was allocated
with devm_kzalloc(). It calls adv7511_cec_init(), which fails for an
unknown reason, which in turn triggers the registered structure to be
removed.

Elsewhere, kirin_drm_platform_probe() gets called, which calls
of_graph_get_remote_node(), and that returns NULL. Before Dan's
patch we would go on with a NULL pointer here and register that,
now kirin_drm_platform_probe() fails with -ENODEV.

In a third driver, dsi_parse_dt() calls drm_of_find_panel_or_bridge(),
which after not finding a panel goes on to call of_drm_find_bridge(),
and that crashes due to the earlier list corruption.

This addresses the first issue by making sure that adv7511_probe()
does not leave behind any corrupted list entries. This should
get the system back to boot but needs testing. From my understanding,
there is at least one more bug that needs to be resolved to actually
get everything to work again.

Reported-by: Naresh Kamboju <naresh.kamboju@...aro.org>
Cc: Xinliang Liu <xinliang.liu@...aro.org>
Cc: Dan Carpenter <dan.carpenter@...cle.com>
Cc: Sean Paul <seanpaul@...omium.org>
Cc: Hans Verkuil <hans.verkuil@...co.com>
Cc: Archit Taneja <architt@...eaurora.org>
Link: https://bugs.linaro.org/show_bug.cgi?id=3345
Link: https://lkft.validation.linaro.org/scheduler/job/48017#L3551
Fixes: 7af35b0addbc ("drm/kirin: Checking for IS_ERR() instead of NULL")
Fixes: 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support")
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
Untested so far, this is what I came up with after reading the
WARN_ON log from a modified kernel.

Naresh, can you give this one a go?

Hans and others, can you review in the meantime?

Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 0e14f1572d05..93d1ecafe8fa 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1204,7 +1204,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 #ifdef CONFIG_DRM_I2C_ADV7511_CEC
 	ret = adv7511_cec_init(dev, adv7511, offset);
 	if (ret)
-		goto err_unregister_cec;
+		goto err_unregister_bridge;
 #else
 	regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
 		     ADV7511_CEC_CTRL_POWER_DOWN);
@@ -1212,6 +1212,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)

 	return 0;

+#ifdef CONFIG_DRM_I2C_ADV7511_CEC
+err_unregister_bridge:
+	adv7511_audio_exit(adv7511);
+	drm_bridge_remove(&adv7511->bridge);
+#endif
 err_unregister_cec:
 	i2c_unregister_device(adv7511->i2c_cec);
 	if (adv7511->cec_clk)
-- 
2.9.0