linux-kernel - Re: [PATCH] soc: qcom: rpmh: Avoid accessing freed memory from batch API

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <154697251815.15366.10888159477746216145@swboyd.mtv.corp.google.com>
Date:   Tue, 08 Jan 2019 10:35:18 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Evan Green <evgreen@...omium.org>, Lina Iyer <ilina@...eaurora.org>
Cc:     Andy Gross <andy.gross@...aro.org>, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        "Raju P.L.S.S.S.N" <rplsssn@...eaurora.org>,
        Matthias Kaehlcke <mka@...omium.org>
Subject: Re: [PATCH] soc: qcom: rpmh: Avoid accessing freed memory from batch API

Quoting Lina Iyer (2019-01-07 13:23:21)
> On Fri, Jan 04 2019 at 14:02 -0700, Evan Green wrote:
> >On Thu, Jan 3, 2019 at 9:47 AM Stephen Boyd <swboyd@...omium.org> wrote:
> >>
> >> @@ -380,7 +386,10 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
> >>         }
> >>
> >>         for (i = 0; i < count; i++) {
> >> -               rpm_msgs[i].completion = &compl;
> >> +               struct completion *compl = &compls[i];
> >> +
> >> +               init_completion(compl);
> >> +               rpm_msgs[i].completion = compl;
> >>                 ret = rpmh_rsc_send_data(ctrlr_to_drv(ctrlr), &rpm_msgs[i].msg);
> >>                 if (ret) {
> >>                         pr_err("Error(%d) sending RPMH message addr=%#x\n",
> >
> >It's a little weird that we call rpmh_tx_done on a bunch of transfers
> >we never submitted, just so the completion will get signaled so we can
> >wait on it in the next loop. We could just do count = i; break; here
> >instead.
> >
> It seems like it was carried over from my earlier submissions, where I
> was reference counting the number of completions for a batch. I beleive,
> with what we are doing here, we don't need to call tx_done with this
> approach.

Ok. So we can remove this whole chunk of code that forces out
completions and unwind more properly?