linux-kernel - Re: [PATCH v8 08/10] drivers: qcom: rpmh: allow requests to be sent asynchronously

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]

Date:   Wed, 23 May 2018 18:00:40 +0530
From:   Raju P L S S S N <rplsssn@...eaurora.org>
To:     Doug Anderson <dianders@...omium.org>,
        Lina Iyer <ilina@...eaurora.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        linux-arm-msm@...r.kernel.org,
        "open list:ARM/QUALCOMM SUPPORT" <linux-soc@...r.kernel.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        msivasub@...eaurora.org, mkshah@...eaurora.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Evan Green <evgreen@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>
Subject: Re: [PATCH v8 08/10] drivers: qcom: rpmh: allow requests to be sent
 asynchronously

Hi,

On 5/12/2018 1:46 AM, Doug Anderson wrote:
> Hi,
> 
> On Wed, May 9, 2018 at 10:01 AM, Lina Iyer <ilina@...eaurora.org> wrote:
>>   /**
>> @@ -137,6 +140,8 @@ void rpmh_tx_done(const struct tcs_request *msg, int r)
>>                  dev_err(rpm_msg->dev, "RPMH TX fail in msg addr=%#x, err=%d\n",
>>                          rpm_msg->msg.cmds[0].addr, r);
>>
>> +       kfree(rpm_msg->free);
>> +
> 
> The way the code is written makes it seem like you could free memory
> _and_ have a completion but you can't.  Specifically:
> 
> * The only plausible thing that "rpm_msg->free" could point to is "rpm_msg".
> 
> * The complete(compl) would then be accessing freed memory.

As the completions are declared on stack, it will not access freed memory.

> 
> I believe the kfree() should be at the end of the function.
> Personally I'd make it more obvious that this is just a boolean value
> and change to:
> 
> if (rpm_msg->needs_free)
>    kgree(rpm_msg)
> 
> ...then the reader of the code doesn't need to go figure out what
> you're trying to free.
> 
> 
> -Doug
> 

Yes. Will change it this way in next patch set.

Thanks,
Raju