[PATCH] drm/bridge: tda998x: Handle short CEC register reads

[PATCH] drm/bridge: tda998x: Handle short CEC register reads

[Ruoyu Wang]

cec_read() uses a two-message I2C transfer to select a register and read
its value. i2c_transfer() returns a negative errno or the number of
messages executed. A short transfer can therefore return zero or one
without writing the second message's buffer.

The current negative-only check treats that as success and returns an
uninitialized stack byte. The value may then affect register
read-modify-write operations, interrupt and HPD handling, or connector
detection.

Require both messages to complete before using the byte. Preserve the
existing zero fallback for failed reads.

This issue was found by a static analysis checker and confirmed by
manual source review.

Fixes: 14e5b5889d75 ("drm/i2c: tda998x: allow sharing of the CEC device accesses")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/gpu/drm/bridge/tda998x_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tda998x_drv.c b/drivers/gpu/drm/bridge/tda998x_drv.c
index 6c427bc75896b..7e82c918277fa 100644
--- a/drivers/gpu/drm/bridge/tda998x_drv.c
+++ b/drivers/gpu/drm/bridge/tda998x_drv.c
@@ -438,7 +438,7 @@ cec_read(struct tda998x_priv *priv, u8 addr)
 	int ret;
 
 	ret = i2c_transfer(priv->hdmi->adapter, msg, ARRAY_SIZE(msg));
-	if (ret < 0) {
+	if (ret != ARRAY_SIZE(msg)) {
 		dev_err(&priv->hdmi->dev, "Error %d reading from cec:0x%x\n",
 			ret, addr);
 		val = 0;
-- 
2.51.0

Re: [PATCH] drm/bridge: tda998x: Handle short CEC register reads

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Dead code in `cec_enamods()` error check allows overwriting the register configuration when `cec_read()` fails.
--

--- Patch [1]: [PATCH] drm/bridge: tda998x: Handle short CEC register reads ---
commit 2bfd5ce7155e5841764b8ec506e16365cd7190b6
Author: Ruoyu Wang <ruoyuw560@gmail.com>

drm/bridge: tda998x: Handle short CEC register reads

This commit updates the error handling in cec_read() to ensure both messages
of the I2C transfer complete successfully before using the returned byte. If
the transfer is short or fails, it preserves the 0 fallback value to avoid
returning uninitialized stack memory.

> diff --git a/drivers/gpu/drm/bridge/tda998x_drv.c b/drivers/gpu/drm/bridge/tda998x_drv.c
> index 0b966c9844e70..fb414584962b7 100644
> --- a/drivers/gpu/drm/bridge/tda998x_drv.c
> +++ b/drivers/gpu/drm/bridge/tda998x_drv.c
> @@ -438,7 +438,7 @@ cec_read(struct tda998x_priv *priv, u8 addr)
>  	int ret;
>  
>  	ret = i2c_transfer(priv->hdmi->adapter, msg, ARRAY_SIZE(msg));
> -	if (ret < 0) {
> +	if (ret != ARRAY_SIZE(msg)) {
>  		dev_err(&priv->hdmi->dev, "Error %d reading from cec:0x%x\n",
>  			ret, addr);
>  		val = 0;
>  	}
>  
>  	return val;

[Severity: High]
This is a pre-existing issue, but since cec_read() returns a u8, does the
error handling in callers like cec_enamods() work correctly?

For example, in drivers/gpu/drm/bridge/tda998x_drv.c:cec_enamods():

	int val = cec_read(priv, REG_CEC_ENAMODS);

	if (val < 0)
		return;

	if (enable)
		val |= mods;
	else
		val &= ~mods;

	cec_write(priv, REG_CEC_ENAMODS, val);

Because cec_read() returns an unsigned 8-bit integer, val will be
zero-extended and can never be negative. If a read fails due to an I2C error,
it returns 0, which bypasses the val < 0 check.

Could this result in writing back a modified 0 to REG_CEC_ENAMODS and
wiping out other enable bits, potentially causing the CEC module or clocks
to be disabled unexpectedly until a full reset occurs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814133952.1386158-1-ruoyuw560@gmail.com?part=1