All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>, xen-devel@lists.xenproject.org
Cc: Lars Kurth <lars.kurth@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Anshul Makkar <anshul.makkar@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	security@xenproject.org, Jan Beulich <jbeulich@suse.com>
Subject: Re: [PATCH] features: declare the Credit2 scheduler as Supported.
Date: Tue, 25 Oct 2016 10:18:52 +0200	[thread overview]
Message-ID: <1477383532.27407.27.camel@citrix.com> (raw)
In-Reply-To: <6064e22e-5c72-0b5e-dcde-98252d9a1cc2@citrix.com>


[-- Attachment #1.1: Type: text/plain, Size: 5086 bytes --]

On Fri, 2016-10-14 at 11:42 +0100, George Dunlap wrote:
> On 14/10/16 11:17, Dario Faggioli wrote:
> > 
> > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> We don't have a general framework for declaring things "supported"
> yet,
> and at the moment we only have a single level of "supported", which
> includes XSAs.  I do think it makes sense to include an assessment of
> the security risk when deciding whether to make such a declaration.
> 
Right... So, first of all, sorry if I dropped the ball on this for a
while, but I really wasn't sure of what should have come next (more
people from security team speaking their mind? Here or somewhere else?
Etc.)

As already said by others, it's hard to follow the steps of a procedure
which is, in fact, unknown and not formalized nor written down anywhere
(which, literally, _just_ mean we need to formalize and write down it!
:-D).

> Here's a sort of checklist we could use to start a discussion.
> 
Ok, thanks George. I'll go through the checklist, and your analysis of
Credit2, and provide my view (when worthwhile).

> 1. Guest -> host privilege escalation.
> 2. Guest user -> guest kernel escalation.
> 3. Information leakage.
> 4. Denial-of-service.
> 
> ---
> 
> WRT Credit2:
> 
> I *think* the only interfaces exposed to *unprivileged* guests are
> the
> SCHEDOP hypercalls -- block(), yield(), &c -- and timers.  (Dario,
> please correct me if I'm wrong.)  
>
I agree, that's all it's there, especially if we are talking about
unprivileged guests. If we include the control domain, we also expose:
 - XEN_DOMCTL_SCHEDOP_* hypercalls,
 - XEN_SYSCTL_SCHEDOP_* hypercalls.

> The only thing that the scheduler
> gives is time on the cpu.  Neither block nor yield contain any
> pointers,
> and so it should be trivial to verify that they contain no privilege
> escalation paths.
> 
I agree.

FWIW, I've tried auditing that code, and could not find anything that
looks like a potential security issue. E.g., there's not buffer/local
variable, being used, that could lead to hypervisor stack leaks (and in
fact, this is probably more about George's point 3 than 1).

> The only information which the scheduler exposes to unprivileged
> guests
> is the timing information.  This may be able to be used for side-
> channel
> attacks to probabilistically infer things about other vcpus running
> on
> the same system; but this has not traditionally been considered
> within
> the security boundary.
> 
And this is possible with all schedulers. Not that we can't think of
improvements, of course, but if this counts, Credit1 is *Experimental*
too. :-)

> There have been denial-of-service attacks on credit1 before, where a
> vcpu could "game the system" by going to sleep at particular times to
> fool the accounting system into thinking that it wasn't running, and
> thus run at BOOST priority and monopolize 95% of the cpu.  But this
> was
> fixed by actually charging cpus for time they spent running rather
> than
> probabilistycally, which credit2 already does.
> 
And Credit2 does not have BOOST or, said in a more general way, it does
not have any way for a vcpu to acquire any kind of "superpowers"...
they're all subjected to the same crediting algorithm, which is a lot
simpler than Credit1's one, and hence a lot easier to understand, debug
and audit.

> It's worth taking stock again and thinking if there's any way to
> "game"
> credit2 in such a way; but I think it's relatively unlikely that
> someone
> will be able to monopolize the cpu 100% as with the credit1 bug; and
> even if it did, it's a pretty low-profile XSA.
> 
It may be me not being a 'security guy' (yet :-)), but I can't think of
anything.

> But I agree with Jan, that such an argument would go well to go in
> the
> commit message. :-)
> 
Right. Actually, I wrote a few words about a lot of development and
testing having been done lately, and I thought whether or not to use
that as commit message. I certainly could have thought about doing
something like this that George did, but:
 - I probably wouldn't have been able to do it as good as him;
 - I wasn't sure whether it was worth/meaningful for me to do it, as 
   opposed to the security team wanting to do that themselves.

So, again, let's formalize the process, because it's hard to follow an
unwritten and implicite set of rules. :-)

All this being said, what's the next step? Is there anything more I
should do, here in this thread? Should we wait for other people to
weigh in? Should I resend this patch, with a non empty commit message?
Containing what, a summary of this?

Any help and insight, would be greatly appreciated.

Thanks for everyone's time and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2016-10-25  8:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-14 10:17 [PATCH] features: declare the Credit2 scheduler as Supported Dario Faggioli
2016-10-14 10:25 ` Jan Beulich
2016-10-14 10:42 ` George Dunlap
2016-10-25  8:18   ` Dario Faggioli [this message]
2016-10-27 15:01     ` George Dunlap
2016-10-27 15:21       ` Dario Faggioli
2016-11-02 10:33       ` Dario Faggioli

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=1477383532.27407.27.camel@citrix.com \
    --to=dario.faggioli@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anshul.makkar@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=lars.kurth@citrix.com \
    --cc=security@xenproject.org \
    --cc=sstabellini@kernel.org \
    --cc=wei.liu2@citrix.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.