From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
"Paul Durrant" <paul@xen.org>,
"Tamas K Lengyel" <tamas@tklengyel.com>,
Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3] x86/mm: Short circuit damage from "fishy" ref/typecount failure
Date: Tue, 19 Jan 2021 17:48:40 +0100 [thread overview]
Message-ID: <98f64276-ec5d-7242-f10f-126fe7ee1f7e@suse.com> (raw)
In-Reply-To: <20210119130254.27058-1-andrew.cooper3@citrix.com>
On 19.01.2021 14:02, Andrew Cooper wrote:
> This code has been copied in 3 places, but it is problematic.
>
> All cases will hit a BUG() later in domain teardown, when a the missing
> type/count reference is underflowed.
I'm afraid I could use some help with this: Why would there
be a missing reference, when the getting of one failed? IOW
I'm not (yet) convinced you don't make the impact more
severe in the (supposedly) impossible case of these paths
getting hit, by converting a domain crash into a host one.
It is in particular relevant that a PV guest may be able to
cheat and "guess" an MFN to obtain references for before a
certain hypercall (or other operation) actually completed.
> v2:
> * Reword the commit message.
> * Switch BUG() to BUG_ON() to further reduce code volume.
Short of us explicitly agreeing that such is fine to use, I
think we ought to stick to the previously (long ago) agreed
rule that BUG_ON() controlling expressions should not have
side effects, for us not wanting to guarantee it will now
and forever _not_ behave like ASSERT() wrt to evaluating
the expression.
Jan
next prev parent reply other threads:[~2021-01-19 18:39 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-19 9:41 [PATCH] x86/mm: Remove cascade damage from "fishy" ref/typecount failure Andrew Cooper
2021-01-19 11:34 ` Andrew Cooper
2021-01-19 12:27 ` [PATCH v2] x86/mm: Short circuit " Andrew Cooper
2021-01-19 12:45 ` Paul Durrant
2021-01-19 13:00 ` Andrew Cooper
2021-01-19 13:02 ` [PATCH v3] " Andrew Cooper
2021-01-19 13:06 ` Paul Durrant
2021-01-19 16:48 ` Jan Beulich [this message]
2021-01-19 18:09 ` Andrew Cooper
2021-01-20 8:06 ` Jan Beulich
2021-01-25 17:59 ` Andrew Cooper
2021-01-26 10:48 ` Jan Beulich
2021-01-28 14:48 ` Jan Beulich
2021-01-29 11:29 ` Jan Beulich
2021-01-29 16:17 ` Andrew Cooper
2021-01-29 16:31 ` Jan Beulich
2021-01-29 17:17 ` Andrew Cooper
2021-02-01 12:50 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98f64276-ec5d-7242-f10f-126fe7ee1f7e@suse.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=paul@xen.org \
--cc=roger.pau@citrix.com \
--cc=tamas@tklengyel.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).