Re: [Xen-devel] [PATCH v2 3/5] tmem: By default to join an shared pool it must be authorized.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Apr 05, 2017 at 03:36:51AM -0600, Jan Beulich wrote:
> >>> On 04.04.17 at 21:10, <konrad.wilk@xxxxxxxxxx> wrote:
> > @@ -1530,7 +1529,8 @@ int do_tmem_new_pool(domid_t this_cli_id,
> >              pool->shared = 0;
> >              goto out;
> >          }
> > -        if ( client->shared_auth_required && !tmem_global.shared_auth )
> > +        /* By default only join domains that are authorized by admin. */
> > +        if ( !tmem_global.shared_auth )
> 
> Why "by default"? Is this comment really useful here? Other than

Took the comment out.
> that the patch looks okay, but I won't claim to understand enough
> of tmem to know this is sufficiently backwards compatible, so I
> won't claim to have reviewed it in full.

The old clients that used shared pools work just fine. That is as long
as the system admin invokes:
        xl tmem-shared-auth  -u 00000000-0000-0000-0000-0000deadbeef -A 1 
<domain>

before hand (this is for UUID 0:deadbeef).
[And to be honest the API is a bit weird - if you can't join a shared
pool then you still get to join a private pool without any errors?!]


Before this change you didn't have to invoke this tmem-shared-auth
and any guest could join a shared pool, even malicious ones.
From that perspective I did break backwards compatibility, but fixed
a security hole.

But as said - the guest won't notice - if the system admin didn't invoke
the tmem-shared-auth - the hypervisor will gladly create another pool
for them, it just that it won't be shared.

> 
> Jan
> 

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