linux-kernel - Re: [PATCH] xen/events: Fix race in set_evtchn_to

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

Message-ID: <361b90c3ec8dc89fa67fb7795d20400f45b53f85.camel@amazon.de>
Date:   Thu, 12 Aug 2021 12:02:12 +0000
From:   "Heyne, Maximilian" <mheyne@...zon.de>
To:     "boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>
CC:     "jbeulich@...e.com" <jbeulich@...e.com>,
        "jgross@...e.com" <jgross@...e.com>,
        "sstabellini@...nel.org" <sstabellini@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Shah, Amit" <aams@...zon.de>,
        "david.vrabel@...rix.com" <david.vrabel@...rix.com>,
        "malcolm.crossley@...rix.com" <malcolm.crossley@...rix.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH] xen/events: Fix race in set_evtchn_to_irq

On Wed, 2021-08-11 at 11:05 -0400, Boris Ostrovsky wrote:
> On 8/11/21 10:08 AM, Maximilian Heyne wrote:
> > This patch sets evtchn_to_irq rows via a cmpxchg operation so that
> > they
> > will be set only once. Clearing the row was moved up before writing
> > the
> > row to evtchn_to_irq in order to not create a race once the row is
> > visible for other threads. Accesses to the rows are now guarded by
> > READ_ONCE and WRITE_ONCE just as for the columns in the data
> > structure.
> 
> Is this last part really needed? We needed to do that for array
> elements to avoid an interrupt handler from seeing a partially
> updated entry but I am not sure I see how this can happen to the row
> pointer. The only place where it might be important is when we update
> the pointer to the new page but you are using cmpxchg there already.

I think you are right. I will remove the changes related to the
READ_ONCE.

> 
> 
> > -             evtchn_to_irq[row] = (int
> > *)get_zeroed_page(GFP_KERNEL);
> > -             if (evtchn_to_irq[row] == NULL)
> > +             evtchn_row = (int *) get_zeroed_page(GFP_KERNEL);
> 
> Not directly related to this patch but I don't think we need to get a
> zeroed page --- we will initialize it to -1 immediately below.

That is correct. I will just fix this in the next version of the patch.

Max



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879