linux-kernel - Re: [PATCH 09/17] soundwire: stream: remove unnecessary variable initializations

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

Message-ID: <346425e0-0001-ecbc-c79a-c1372d0b4fcc@linux.intel.com>
Date:   Tue, 6 Aug 2019 10:39:23 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Cezary Rojewski <cezary.rojewski@...el.com>
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        tiwai@...e.de, broonie@...nel.org, vkoul@...nel.org,
        gregkh@...uxfoundation.org, jank@...ence.com,
        srinivas.kandagatla@...aro.org, Blauciak@...r.kernel.org,
        Slawomir <slawomir.blauciak@...el.com>,
        Sanyog Kale <sanyog.r.kale@...el.com>
Subject: Re: [PATCH 09/17] soundwire: stream: remove unnecessary variable
 initializations



On 8/6/19 10:31 AM, Cezary Rojewski wrote:
> On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
>> @@ -1493,6 +1493,11 @@ static int _sdw_prepare_stream(struct 
>> sdw_stream_runtime *stream)
>>           }
>>       }
>> +    if (!bus) {
>> +        pr_err("Configuration error in %s\n", __func__);
>> +        return -EINVAL;
>> +    }
>> +
> 
> This should probably be located in separate path - not at all an 
> initialization removal.

It's a consequence of the initialization removal: because we are 
removing the default init, there is a risk that the loop just before do 
not set it, so it's required to trap the case where the variable in not 
initialized.

> 
>> @@ -1573,6 +1578,11 @@ static int _sdw_enable_stream(struct 
>> sdw_stream_runtime *stream)
>>           }
>>       }
>> +    if (!bus) {
>> +        pr_err("Configuration error in %s\n", __func__);
>> +        return -EINVAL;
>> +    }
>> +
> 
> Same here.

same reply

> 
>> @@ -1639,13 +1650,14 @@ static int _sdw_disable_stream(struct 
>> sdw_stream_runtime *stream)
>>       ret = do_bank_switch(stream);
>>       if (ret < 0) {
>> -        dev_err(bus->dev, "Bank switch failed: %d\n", ret);
>> +        pr_err("Bank switch failed: %d\n", ret);
>>           return ret;
>>       }
> 
> Here too.

no, same thing, the bus variable is initialized in loops so tools will 
report a possible path where bus->dev is an invalid dereference.

> I might have missed something though I bet you got my point.