Re: [PATCH 2/2] xen/flask: Wire up XEN_DOMCTL_{get,set}_paging_mempool_size

From: Jason Andryuk <jandryuk@gmail.com>
To: Andrew Cooper <Andrew.Cooper3@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	 Daniel Smith <dpsmith@apertussolutions.com>,
	Henry Wang <Henry.Wang@arm.com>
Subject: Re: [PATCH 2/2] xen/flask: Wire up XEN_DOMCTL_{get,set}_paging_mempool_size
Date: Mon, 21 Nov 2022 10:54:15 -0500	[thread overview]
Message-ID: <CAKf6xps3OQsuFMNOnJ1hoMvk5tnkNpUhPdNRqdPeht9dTvS+jA@mail.gmail.com> (raw)
In-Reply-To: <9a302c19-4f07-77a5-5e41-310498c7cf23@citrix.com>

On Mon, Nov 21, 2022 at 10:46 AM Andrew Cooper
<Andrew.Cooper3@citrix.com> wrote:
>
> On 21/11/2022 15:39, Jason Andryuk wrote:
> > On Mon, Nov 21, 2022 at 9:37 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> These were overlooked in the original patch, and noticed by OSSTest which does
> >> run some Flask tests.
> >>
> >> Fixes: 22b20bd98c02 ("xen: Introduce non-broken hypercalls for the paging mempool size")
> >> Suggested-by: Daniel Smith <dpsmith@apertussolutions.com>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> >> CC: Daniel Smith <dpsmith@apertussolutions.com>
> >> CC: Jason Andryuk <jandryuk@gmail.com>
> >> CC: Henry Wang <Henry.Wang@arm.com>
> > Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> >
> > Thanks, Andrew.  Though we might want a small tweak - possibly as a follow up?
> >
> >> diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
> >> index 424daab6a022..6b7b7d403ab4 100644
> >> --- a/tools/flask/policy/modules/xen.if
> >> +++ b/tools/flask/policy/modules/xen.if
> >> @@ -92,7 +92,7 @@ define(`manage_domain', `
> >>         allow $1 $2:domain { getdomaininfo getvcpuinfo getaffinity
> >>                         getaddrsize pause unpause trigger shutdown destroy
> >>                         setaffinity setdomainmaxmem getscheduler resume
> >> -                       setpodtarget getpodtarget };
> >> +                       setpodtarget getpodtarget getpagingmempool setpagingmempool };
> > There is also create_domain_common which is for a dedicated "domain
> > builder" that creates but does not manage domains.  I think that
> > should gain setpagingmempool permission?
>
> Sounds like it should.  Something like this?
>
> diff --git a/tools/flask/policy/modules/xen.if
> b/tools/flask/policy/modules/xen.if
> index 6b7b7d403ab4..11c1562aa5da 100644
> --- a/tools/flask/policy/modules/xen.if
> +++ b/tools/flask/policy/modules/xen.if
> @@ -49,7 +49,8 @@ define(`create_domain_common', `
>         allow $1 $2:domain { create max_vcpus setdomainmaxmem setaddrsize
>                         getdomaininfo hypercall setvcpucontext getscheduler
>                         getvcpuinfo getaddrsize getaffinity setaffinity
> -                       settime setdomainhandle getvcpucontext
> set_misc_info };
> +                       settime setdomainhandle getvcpucontext set_misc_info
> +                       getpagingmempool setpagingmempool };
>         allow $1 $2:domain2 { set_cpu_policy settsc setscheduler setclaim
>                         set_vnumainfo get_vnumainfo cacheflush
>                         psr_cmt_op psr_alloc soft_reset
>
> I can fold this in on commit.

Yes, though strictly speaking it may only need setpagingmempool and
not getpagingmempool.  These are all calls that would execute before
the domain is run.  But adding both is probably fine since the builder
is setting these itself.  i.e. allowing it to read what it set is
fine.

So, yes, I'd just fold this in.

Thanks,
Jason