From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161276AbcIWS3F (ORCPT ); Fri, 23 Sep 2016 14:29:05 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:43575 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034572AbcIWS3E (ORCPT ); Fri, 23 Sep 2016 14:29:04 -0400 Subject: Re: [PATCH] greybus: audio: ensure module is set to avoid crash on dev_err message To: Vaibhav Agarwal , Greg Kroah-Hartman References: <20160923102540.18261-1-colin.king@canonical.com> <20160923165802.GA7575@kroah.com> Cc: Johan Hovold , Alex Elder , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org From: Colin Ian King Message-ID: <57E5746D.6020606@canonical.com> Date: Fri, 23 Sep 2016 19:29:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 23/09/16 19:20, Vaibhav Agarwal wrote: > On Fri, Sep 23, 2016 at 10:28 PM, Greg Kroah-Hartman > wrote: >> On Fri, Sep 23, 2016 at 11:25:40AM +0100, Colin King wrote: >>> From: Colin Ian King >>> >>> Currently, if info is null, the dev_err message is dereferencing an >>> uninitialized module pointer. Instead, initialize module before the >>> dev_err call to fix this issue. >>> >>> Found using static analysis with cppcheck: >>> [drivers/staging/greybus/audio_topology.c:175]: (error) >>> Uninitialized variable: module >>> >>> Signed-off-by: Colin Ian King >>> --- >>> drivers/staging/greybus/audio_topology.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c >>> index 5eef536..c43a959 100644 >>> --- a/drivers/staging/greybus/audio_topology.c >>> +++ b/drivers/staging/greybus/audio_topology.c >>> @@ -171,6 +171,9 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol *kcontrol, >>> data = (struct gbaudio_ctl_pvt *)kcontrol->private_value; >>> info = (struct gb_audio_ctl_elem_info *)data->info; >>> >>> + module = find_gb_module(gbcodec, kcontrol->id.name); >>> + if (!module) >>> + return -EINVAL; >> >> How do you know you can get a module at this point in time, you haven't >> looked at the type of info to know that? >> >> I agree that there is an issue here, but I don't think this is the >> correct fix. Vaibhav? > > Actually, fetching module is not related to info->type. However it is > only required in case of ENUMERATED element_type, thus used in > specific case. Also, I think it's better to use codec->dev in the err > message and align with other err_msg in this function. > > Hi Colin, thanks for sharing this patch. Do you mind updating this > patch including my above suggestion? Otherwise, I can share a separate > patch. Vaibhav, I'm currently traveling, so won't be able to check this until next week, so if you would rather send a correct fix sooner rather me re-sending a fix then I'm fine with that. Thanks, Colin > > -- > thanks, > vaibhav >>