Re: [PATCH] flask: label-pci: Allow specifying optional irq label

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 3/13/23 13:50, Jason Andryuk wrote:
> IRQs can be shared between devices, so using the same label as the PCI
> device can create conflicts where the IRQ is labeled with one of the
> device labels preventing assignment of the second device to the second
> domain.  Add the ability to specify an irq label distinct from the PCI
> device, so a shared irq label can be specified.  The policy would then
> be written such that the two domains can each use the shared IRQ type in
> addition to their labeled PCI device.  That way we can still label most
> of the PCI device resources and assign devices in the face of shared
> IRQs.
>
> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> Reviewed-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> ---
> v2:
> Describe usage in docs/misc/xsm-flask.txt
> ---
>   docs/misc/xsm-flask.txt       | 16 ++++++++++++++++
>   tools/flask/utils/label-pci.c | 13 ++++++++++---
>   2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
> index 2419c5cf29..ba89ebbfd8 100644
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -205,6 +205,22 @@ parameter, which can also be changed using xl setenforce). 
>  When using the
>   default types for domains (domU_t), the example policy shipped with Xen should
>   allow the same operations on or between domains as when not using FLASK.
>   
> +By default, flask-label-pci labels the device, I/O ports, memory and IRQ with
> +the provided label.  These are all unique per-device, except for IRQs which
> +can be shared between devices.  This leads to assignment problems since vmA_t
> +can't access the IRQ devB_t.  To work around this issue, flask-label-pci
> +takes an optional 3rd argument to label the IRQ:
> +
> +    flask-label-pci 0000:03:02.0 system_u:object_r:nic_dev_t \
> +        system_u:object_r:shared_irq_t
> +
> +The IRQ labeling only applies to the PIRQ - MSI/MSI-X interrupts are labeled
> +with the main device label.
> +
> +The policy needs to define the shared_irq_t with:
> +    type shared_irq_t, resource_type;
> +
> +And the policy needs to be updated to allow domains appropriate access.
>   
>   MLS/MCS policy
>   --------------
> diff --git a/tools/flask/utils/label-pci.c b/tools/flask/utils/label-pci.c
> index 9ddb713cf4..897b772804 100644
> --- a/tools/flask/utils/label-pci.c
> +++ b/tools/flask/utils/label-pci.c
> @@ -28,7 +28,7 @@
>   
>   static void usage (int argCnt, char *argv[])
>   {
> -       fprintf(stderr, "Usage: %s SBDF label\n", argv[0]);
> +       fprintf(stderr, "Usage: %s SBDF label <irq_label>\n", argv[0]);
>         exit(1);
>   }
>   
> @@ -39,12 +39,19 @@ int main (int argCnt, char *argv[])
>         int seg, bus, dev, fn;
>         uint32_t sbdf;
>         uint64_t start, end, flags;
> +       char *pirq_label;
>         char buf[1024];
>         FILE *f;
>   
> -	if (argCnt != 3)
> +       if (argCnt < 3 || argCnt > 4)
style nit: space inside parens

>                 usage(argCnt, argv);
>   
> +	if (argCnt == 4) {
> +           pirq_label = argv[3];
> +       } else {
> +           pirq_label = argv[2];
> +       }
> +
style nit: space inside parens and curly brackets could be dropped or 
should be moved to their own lines.
>         xch = xc_interface_open(0,0,0);
>         if ( !xch )
>         {
> @@ -107,7 +114,7 @@ int main (int argCnt, char *argv[])
>         if (fscanf(f, "%" SCNu64, &start) != 1)
>                 start = 0;
>         if (start) {
> -               ret = xc_flask_add_pirq(xch, start, argv[2]);
> +               ret = xc_flask_add_pirq(xch, start, pirq_label);
>                 if (ret) {
>                         fprintf(stderr, "xc_flask_add_pirq %"PRIu64" failed: 
> %d\n",
>                                         start, ret);
Style nits aside, LGTM.

Acked-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>