Skip to contentSkip to conversation

media: omap3isp: zero-initialize the isp cam_xclk{a,b} initial data

PATCH In review Javier Martinez Canillas 2 participants 2 messages Jun 9, 2018 – Jun 10, 2018 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 media: omap3isp: zero-initialize the isp cam_xclk{a,b} initial data

The struct clk_init_data init variable is declared in the isp_xclk_init() function so is an automatic variable allocated in the stack. But it's not explicitly zero-initialized, so some init fields are left uninitialized.

This causes the data structure to have undefined values that may confuse the common clock framework when the clock is registered.

For example, the uninitialized .flags field could have the CLK_IS_CRITICAL bit set, causing the framework to wrongly prepare the clk on registration. This leads to the isp_xclk_prepare() callback being called, which in turn calls to the omap3isp_get() function that increments the isp dev refcount.

Since this omap3isp_get() call is unexpected, this leads to an unbalanced omap3isp_get() call that prevents the requested IRQ to be later enabled, due the refcount not being 0 when the correct omap3isp_get() call happens.

Fixes 9b28ee3c9122 ("[media] omap3isp: Use the common clock framework")

---

Changes in v2:
- Correct some typos in the commit message.

 drivers/media/platform/omap3isp/isp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/platform/omap3isp/isp.c +1 -1
index f22cf351e3e..ae0ef8b241a 100644
@@ -300,7 +300,7 @@ static struct clk *isp_xclk_src_get(struct of_phandle_args *clkspec, void *data)
 static int isp_xclk_init(struct isp_device *isp)
 {
 	struct device_node *np = isp->dev->of_node;
-	struct clk_init_data init;
+	struct clk_init_data init = { 0 };
 	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i)
Signature · 1 line
2.17.1
Reviewed-by on v2 1/1
#1 lore raw

Hi,

Quoted 12 lines · Javier Martinez Canillas

The struct clk_init_data init variable is declared in the isp_xclk_init() function so is an automatic variable allocated in the stack. But it's not explicitly zero-initialized, so some init fields are left uninitialized.

This causes the data structure to have undefined values that may confuse the common clock framework when the clock is registered.

For example, the uninitialized .flags field could have the CLK_IS_CRITICAL bit set, causing the framework to wrongly prepare the clk on registration. This leads to the isp_xclk_prepare() callback being called, which in turn calls to the omap3isp_get() function that increments the isp dev refcount.

Quoted 6 lines

Since this omap3isp_get() call is unexpected, this leads to an unbalanced omap3isp_get() call that prevents the requested IRQ to be later enabled, due the refcount not being 0 when the correct omap3isp_get() call happens.

Fixes 9b28ee3c9122 ("[media] omap3isp: Use the common clock framework")

good catch!

-- Sebastian

Quoted 25 lines

---

Changes in v2:
- Correct some typos in the commit message.

 drivers/media/platform/omap3isp/isp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/platform/omap3isp/isp.c +1 -1
index f22cf351e3e..ae0ef8b241a 100644
@@ -300,7 +300,7 @@ static struct clk *isp_xclk_src_get(struct of_phandle_args *clkspec, void *data)
 static int isp_xclk_init(struct isp_device *isp)
 {
 	struct device_node *np = isp->dev->of_node;
-	struct clk_init_data init;
+	struct clk_init_data init = { 0 };
 	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i)
Signature · 1 line
2.17.1