linux-kernel - Re: [PATCH v2] media: omap3isp: zero-initialize the isp cam

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-prev] [day] [month] [year] [list]

Message-ID: <20180610000535.32ouofio7n5b6rmj@earth.universe>
Date:   Sun, 10 Jun 2018 02:05:35 +0200
From:   Sebastian Reichel <sre@...nel.org>
To:     Javier Martinez Canillas <javierm@...hat.com>
Cc:     linux-kernel@...r.kernel.org,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        linux-media@...r.kernel.org
Subject: Re: [PATCH v2] media: omap3isp: zero-initialize the isp
 cam_xclk{a,b} initial data

Hi,

On Sat, Jun 09, 2018 at 02:22:45PM +0200, Javier Martinez Canillas wrote:
> 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")
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>

good catch!

Reviewed-by: Sebastian Reichel <sebastian.reichel@...labora.co.uk>

-- Sebastian

> 
> ---
> 
> Changes in v2:
> - Correct some typos in the commit message.
> 
>  drivers/media/platform/omap3isp/isp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index f22cf351e3e..ae0ef8b241a 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -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)
> -- 
> 2.17.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)