[v2] xsm/flask: adjust print messages to use %pd

Commit Message

Daniel P. Smith Sept. 21, 2022, 3:16 p.m. UTC

Print messages from flask use an inconsistent format when printing the domain
id. When referencing system domains, the domain id is printed which is not
immediately identifiable. The %pd conversion specifier provides a consistent
and clear way to format for the domain id. In addition this will assist in
aligning FLASK with current hypervisor code practices.

While addressing the domain id formating, two relatd issues were addressed.
The first being that avc_printk() was not applying any conversion specifier
validation. To address this, the printf annotation was added to avc_printk() to
help ensure the correct types are passed to each conversion specifier. The second
was concern that source and target domains were being appropriately reported for
an AVC. This was addressed by simplifying the conditional logic.

Signed-off-by: Daniel P. Smith <[e-mail]>
Reviewed-by: Henry Wang <[e-mail]>
---

Changes in v2:
 - Grossly simplified the conditional logic for printing source/target domids
   to address concerns raised by Jan.

 xen/xsm/flask/avc.c   | 16 ++++++++--------
 xen/xsm/flask/hooks.c |  3 +--
 2 files changed, 9 insertions(+), 10 deletions(-)

Comments

Jason Andryuk Sept. 21, 2022, 3:39 p.m. UTC | #1

On Wed, Sep 21, 2022 at 11:16 AM Daniel P. Smith
<[e-mail]> wrote:
>
> Print messages from flask use an inconsistent format when printing the domain
> id. When referencing system domains, the domain id is printed which is not
> immediately identifiable. The %pd conversion specifier provides a consistent
> and clear way to format for the domain id. In addition this will assist in
> aligning FLASK with current hypervisor code practices.
>
> While addressing the domain id formating, two relatd issues were addressed.

s/relatd/related/

> The first being that avc_printk() was not applying any conversion specifier
> validation. To address this, the printf annotation was added to avc_printk() to
> help ensure the correct types are passed to each conversion specifier. The second
> was concern that source and target domains were being appropriately reported for
> an AVC. This was addressed by simplifying the conditional logic.
>
> Signed-off-by: Daniel P. Smith <[e-mail]>
> Reviewed-by: Henry Wang <[e-mail]>

Reviewed-by: Jason Andryuk <[e-mail]>

Replacing domid=32767 with source=d[IDLE] seems more user friendly to me.

Thanks,
Jason

Patch

diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index 4a75ec97e2..3d39e55cae 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -113,7 +113,8 @@  struct avc_dump_buf {
     u32 free;
 };
 
-static void avc_printk(struct avc_dump_buf *buf, const char *fmt, ...)
+static void __attribute__ ((format (printf, 2, 3)))
+    avc_printk(struct avc_dump_buf *buf, const char *fmt, ...)
 {
     int i;
     va_list args;
@@ -565,15 +566,14 @@  void avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 requested,
 
     if ( a && (a->sdom || a->tdom) )
     {
-        if ( a->sdom && a->tdom && a->sdom != a->tdom )
-            avc_printk(&buf, "domid=%d target=%d ", a->sdom->domain_id, a->tdom->domain_id);
-        else if ( a->sdom )
-            avc_printk(&buf, "domid=%d ", a->sdom->domain_id);
-        else
-            avc_printk(&buf, "target=%d ", a->tdom->domain_id);
+        if ( a->sdom )
+            avc_printk(&buf, "source=%pd ", a->sdom);
+        if ( a->tdom && a->tdom != a->sdom )
+            avc_printk(&buf, "target=%pd ", a->tdom);
     }
     else if ( cdom )
-        avc_printk(&buf, "domid=%d ", cdom->domain_id);
+        avc_printk(&buf, "current=%pd ", cdom);
+
     switch ( a ? a->type : 0 ) {
     case AVC_AUDIT_DATA_DEV:
         avc_printk(&buf, "device=%#lx ", a->device);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 8bd56644ef..a79281bdb0 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -281,8 +281,7 @@  static int cf_check flask_evtchn_interdomain(
     rc = security_transition_sid(sid1, sid2, SECCLASS_EVENT, &newsid);
     if ( rc )
     {
-        printk("security_transition_sid failed, rc=%d, Dom%d\n",
-               -rc, d2->domain_id);
+        printk("security_transition_sid failed, rc=%d, %pd\n", -rc, d2);
         return rc;
     }