These two arrays are populated with data read from the I2C device
through regmap_read(), and the data is then compared with hardcoded
vendor/product ID values of supported chips.
However, the return value of regmap_read() was never checked. This is
fine, as long as the two arrays are zero-initialized, so that we don't
compare the vendor/product IDs against whatever garbage is left on the
stack.
Address this issue by zero-initializing these two arrays.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 2f2a09adb4bc..b130d01147c6 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -889,7 +889,7 @@ static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
static int it66121_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- u32 vendor_ids[2], device_ids[2], revision_id;
+ u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
struct device_node *ep;
int ret;
struct it66121_ctx *ctx;
--
2.33.0
If run before the next bridge is initialized, of_drm_find_bridge() will
give us a NULL pointer.
If that's the case, return -EPROBE_DEFER; we may have more luck next
time.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index b130d01147c6..9dc41a7b9136 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -924,6 +924,9 @@ static int it66121_probe(struct i2c_client *client,
ctx->next_bridge = of_drm_find_bridge(ep);
of_node_put(ep);
+ if (!ctx->next_bridge)
+ return -EPROBE_DEFER;
+
i2c_set_clientdata(client, ctx);
mutex_init(&ctx->lock);
--
2.33.0
On 27/08/2021 18:39, Paul Cercueil wrote:
> If run before the next bridge is initialized, of_drm_find_bridge() will
> give us a NULL pointer.
>
> If that's the case, return -EPROBE_DEFER; we may have more luck next
> time.
>
Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index b130d01147c6..9dc41a7b9136 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -924,6 +924,9 @@ static int it66121_probe(struct i2c_client *client,
> ctx->next_bridge = of_drm_find_bridge(ep);
> of_node_put(ep);
>
> + if (!ctx->next_bridge)
> + return -EPROBE_DEFER;
> +
> i2c_set_clientdata(client, ctx);
> mutex_init(&ctx->lock);
>
>
Reviewed-by: Neil Armstrong <[email protected]>
Thanks,
Neil
On 27/08/2021 18:39, Paul Cercueil wrote:
> These two arrays are populated with data read from the I2C device
> through regmap_read(), and the data is then compared with hardcoded
> vendor/product ID values of supported chips.
>
> However, the return value of regmap_read() was never checked. This is
> fine, as long as the two arrays are zero-initialized, so that we don't
> compare the vendor/product IDs against whatever garbage is left on the
> stack.
>
> Address this issue by zero-initializing these two arrays.
>
Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 2f2a09adb4bc..b130d01147c6 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -889,7 +889,7 @@ static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
> static int it66121_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - u32 vendor_ids[2], device_ids[2], revision_id;
> + u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
> struct device_node *ep;
> int ret;
> struct it66121_ctx *ctx;
>
Reviewed-by: Neil Armstrong <[email protected]>
Thanks,
Neil
On Mon, 30 Aug 2021 at 11:40, Neil Armstrong <[email protected]> wrote:
>
> On 27/08/2021 18:39, Paul Cercueil wrote:
> > These two arrays are populated with data read from the I2C device
> > through regmap_read(), and the data is then compared with hardcoded
> > vendor/product ID values of supported chips.
> >
> > However, the return value of regmap_read() was never checked. This is
> > fine, as long as the two arrays are zero-initialized, so that we don't
> > compare the vendor/product IDs against whatever garbage is left on the
> > stack.
> >
> > Address this issue by zero-initializing these two arrays.
> >
>
> Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
>
> > Signed-off-by: Paul Cercueil <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/ite-it66121.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> > index 2f2a09adb4bc..b130d01147c6 100644
> > --- a/drivers/gpu/drm/bridge/ite-it66121.c
> > +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> > @@ -889,7 +889,7 @@ static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
> > static int it66121_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > - u32 vendor_ids[2], device_ids[2], revision_id;
> > + u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
> > struct device_node *ep;
> > int ret;
> > struct it66121_ctx *ctx;
> >
>
> Reviewed-by: Neil Armstrong <[email protected]>
Applied series to drm-misc-next.