linux-kernel - RE: [PATCH 02/30] habanalabs: fix race when waiting on encaps signal

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

Message-ID: <PR1PR02MB476183CADA89FEAAB743EAD4AF5E9@PR1PR02MB4761.eurprd02.prod.outlook.com>
Date:   Mon, 24 Jan 2022 18:22:39 +0000
From:   Dani Liberman <dliberman@...ana.ai>
To:     Hillf Danton <hdanton@...a.com>, Oded Gabbay <ogabbay@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 02/30] habanalabs: fix race when waiting on encaps signal



On Sun, 23 Jan 2022 02:27 +0200 Hillf Danton wrote:
> On Sat, 22 Jan 2022 21:57:03 +0200 Oded Gabbay wrote:
> > @@ -2063,13 +2063,22 @@ static int cs_ioctl_signal_wait(struct hl_fpriv
> *hpriv, enum hl_cs_type cs_type,
> >                       idp = &ctx->sig_mgr.handles;
> >                       idr_for_each_entry(idp, encaps_sig_hdl, id) {
> >                               if (encaps_sig_hdl->cs_seq == signal_seq) {
> > -                                     handle_found = true;
> >                                       /* get refcount to protect removing
> >                                        * this handle from idr, needed when
> >                                        * multiple wait cs are used with offset
> >                                        * to wait on reserved encaps signals.
> >                                        */
> >                                       kref_get(&encaps_sig_hdl->refcount);
> > +                                     /*
> > +                                      * Since kref_put of this handle is executed outside the
> > +                                      * current lock, it is possible that the handle refcount
> > +                                      * is 0 but it yet to be removed from the list. In this case
> > +                                      * need to consider the handle as not valid. To ensure
> > +                                      * that the handle is valid, its refcount must be bigger
> > +                                      * than 1.
> > +                                      */
> > +                                     if (kref_read(&encaps_sig_hdl->refcount) > 1)
> > +                                             handle_found = true;
> >                                       break;
> >                               }
> >                       }
> > --
> > 2.25.1
> 
> Wonder why kref_get_unless_zero() does not fit here, given the chance
> for bumping zero refcount up?
> 
> Hillf

Thanks, you are right.
I will send an updated patch with your suggestion.

Dani