Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221109105158.230081-1-zyytlz.wz@163.com> <Y2uLK6Zxsz9TD9WV@kroah.com>
 <CAJedcCyN+D+gsVo10r-bGpAtnL4x_N3TpqhVYBi1P7=dD8fNCw@mail.gmail.com> <Y2uYZFqzYdG/oJCY@kroah.com>
In-Reply-To: <Y2uYZFqzYdG/oJCY@kroah.com>
From:   Zheng Hacker <hackerzheng666@gmail.com>
Date:   Wed, 9 Nov 2022 20:56:31 +0800
Message-ID: <CAJedcCxstTJQiVPpW7sTMRBBPixQMd3HqNAX3S5GeXGjjXDKAQ@mail.gmail.com>
Subject: Re: [PATCH v8] misc: sgi-gru: fix use-after-free error in
 gru_set_context_option, gru_fault and gru_handle_user_call_os
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     Zheng Wang <zyytlz.wz@163.com>, zhengyejian1@huawei.com,
        dimitri.sivanich@hpe.com, arnd@arndb.de,
        linux-kernel@vger.kernel.org, alex000young@gmail.com,
        security@kernel.org, sivanich@hpe.com, lkp@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Greg KH <gregkh@linuxfoundation.org> =E4=BA=8E2022=E5=B9=B411=E6=9C=889=E6=
=97=A5=E5=91=A8=E4=B8=89 20:09=E5=86=99=E9=81=93=EF=BC=9A
>
> On Wed, Nov 09, 2022 at 08:04:04PM +0800, Zheng Hacker wrote:
> > Greg KH <gregkh@linuxfoundation.org> =E4=BA=8E2022=E5=B9=B411=E6=9C=889=
=E6=97=A5=E5=91=A8=E4=B8=89 19:12=E5=86=99=E9=81=93=EF=BC=9A
> > > >       /*
> > > >        * If the current task is the context owner, verify that the
> > > > @@ -727,14 +728,16 @@ void gru_check_context_placement(struct gru_t=
hread_state *gts)
> > > >        */
> > > >       gru =3D gts->ts_gru;
> > > >       if (!gru || gts->ts_tgid_owner !=3D current->tgid)
> > > > -             return;
> > > > +             return ret;
> > >
> > > Why does this check return "all is good!" ?
> > >
> > > Shouldn't that be an error?
> > >
> > This check is something like "if the gts has been initiiazed properly".
> > If it's not, I thinks we shouldn't treat the gts like something very
> > bad happend. Because in the later request, the gts can still have a
> > chance to be configed/updated properly. This is different from "it's
> > too bad so we have to unload gts right now". This is just my personal
> > point of view. Besides, the caller of this function have token it into
> > consider. In gru_fault, it will try again and in gru_handle_user_call_o=
s,
> > it will return -EAGAIN. In gru_set_context_option, it will be fine
> > because there won't be any operation on gts->ts_gru or gts->ts_tgid_own=
er
>
> Then you need to document it why this is "success" as it is not obvious
> at all.
>
Oh yes. I will append a comment to document it with the code in the
next version of patch.

Best regards,
Zheng Wang