Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PATCH] soc: qcom: rpmh: Avoid accessing freed memory from batch API
Cc:     Andy Gross <andy.gross@linaro.org>, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        "Raju P.L.S.S.S.N" <rplsssn@codeaurora.org>,
        Matthias Kaehlcke <mka@chromium.org>
To:     Evan Green <evgreen@chromium.org>, Lina Iyer <ilina@codeaurora.org>
In-Reply-To: <20190107212321.GK14960@codeaurora.org>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.8
References: <20190103174657.251968-1-swboyd@chromium.org> <CAE=gft6M7aaPjS0KRg7dyfLOGtgXdU1qaT7oNeRHeNFQpRouWA@mail.gmail.com> <20190107212321.GK14960@codeaurora.org>
Message-ID: <154697251815.15366.10888159477746216145@swboyd.mtv.corp.google.com>
Date:   Tue, 08 Jan 2019 10:35:18 -0800
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@chromium.org> wrote:
> >>
> >> @@ -380,7 +386,10 @@ int rpmh_write_batch(const struct device *dev, en=
um rpmh_state state,
> >>         }
> >>
> >>         for (i =3D 0; i < count; i++) {
> >> -               rpm_msgs[i].completion =3D &compl;
> >> +               struct completion *compl =3D &compls[i];
> >> +
> >> +               init_completion(compl);
> >> +               rpm_msgs[i].completion =3D compl;
> >>                 ret =3D rpmh_rsc_send_data(ctrlr_to_drv(ctrlr), &rpm_m=
sgs[i].msg);
> >>                 if (ret) {
> >>                         pr_err("Error(%d) sending RPMH message addr=3D=
%#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 =3D 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?