From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S969325AbdAEWkd (ORCPT ); Thu, 5 Jan 2017 17:40:33 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:50934 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S968927AbdAEWkA (ORCPT ); Thu, 5 Jan 2017 17:40:00 -0500 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org 3FDCD60440 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=sboyd@codeaurora.org Date: Thu, 5 Jan 2017 14:39:57 -0800 From: Stephen Boyd To: Vivek Gautam Cc: Andy Gross , mturquette@baylibre.com, david.brown@linaro.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, Georgi Djakov Subject: Re: [PATCH] clk: qcom: Fix a possible NULL pointer dereferencing Message-ID: <20170105223957.GK17126@codeaurora.org> References: <1483606525-27709-1-git-send-email-vivek.gautam@codeaurora.org> <20170105142002.GB5710@hector.attlocal.net> <9b590296-6e3a-9079-08e5-48b4af8f8f22@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9b590296-6e3a-9079-08e5-48b4af8f8f22@codeaurora.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/05, Vivek Gautam wrote: > > On 01/05/2017 07:50 PM, Andy Gross wrote: > >On Thu, Jan 05, 2017 at 02:25:25PM +0530, Vivek Gautam wrote: > >>Assign num_parents as 0 while registering fixed rate clocks > >>in _qcom_cc_register_board_clk(), to make sure the clk framework > >>doesn't dereference parent. > >> > >>Fixes: ee15faffef11 ("clk: qcom: common: Add API to register board clocks backwards compatibly") > >> > >>Cc: Georgi Djakov > >>Signed-off-by: Vivek Gautam > >>--- > >> > >>Based on 'clk-next'. Build tested. > >> > >> drivers/clk/qcom/common.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >>diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c > >>index cfab7b400381..df004ead1bef 100644 > >>--- a/drivers/clk/qcom/common.c > >>+++ b/drivers/clk/qcom/common.c > >>@@ -157,6 +157,7 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path, > >> init_data.name = path; > >> init_data.ops = &clk_fixed_rate_ops; > >>+ init_data.num_parents = 0; > >It seems like there was a initializer in the declaration but it was { } instead > >of { 0 }. > > > >Was the original intent to make this structure initialized to 0? If so, perhaps > >it should be fixed above in the initializer. > > yes, i think we intend to initialize the clock init data to 0, and thus > we should do that during declaration. > Will modify and re-spin the patch. > What's the error exactly? Do you have some call stack/crash that could be put in the commit text? It was my understanding that GCC allows braces without anything inside to initialize structures to 0, so I'm confused what's wrong here. If this is actually a problem then we have other places to fix this. $ git grep "clk_init_data.*{ *}" drivers/clk/bcm/clk-bcm53573-ilp.c: struct clk_init_data init = { }; drivers/clk/clk-gpio.c: struct clk_init_data init = {}; drivers/clk/clk-qoriq.c: struct clk_init_data init = {}; drivers/clk/clk-rk808.c: struct clk_init_data init = {}; drivers/clk/mediatek/clk-apmixed.c: struct clk_init_data init = {}; drivers/clk/mediatek/clk-gate.c: struct clk_init_data init = {}; drivers/clk/mediatek/clk-pll.c: struct clk_init_data init = {}; drivers/clk/qcom/common.c: struct clk_init_data init_data = { }; drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c: struct clk_init_data bytediv_init = { }; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project