On 5/2/23 09:30, Juergen Gross wrote:
On 02.05.23 15:23, Daniel P. Smith wrote:
On 5/2/23 09:13, Juergen Gross wrote:
On 02.05.23 15:03, Daniel P. Smith wrote:
On 4/30/23 10:46, Juergen Gross wrote:
In case XSM is active, the handling of XEN_SYSCTL_getdomaininfolist
can fail if the last domain scanned isn't allowed to be accessed by
the calling domain (i.e. xsm_getdomaininfo(XSM_HOOK, d) is failing).
Fix that by just ignoring scanned domains where xsm_getdomaininfo()
is returning an error, like it is effectively done when such a
situation occurs for a domain not being the last one scanned.
Fixes: d046f361dc93 ("Xen Security Modules: XSM")
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
xen/common/sysctl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 02505ab044..0cbfe8bd44 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -89,8 +89,7 @@ long
do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
if ( num_domains ==
op->u.getdomaininfolist.max_domains )
break;
- ret = xsm_getdomaininfo(XSM_HOOK, d);
- if ( ret )
+ if ( xsm_getdomaininfo(XSM_HOOK, d) )
continue;
getdomaininfo(d, &info);
This change does not match the commit message. This says it fixes an
issue, but unless I am totally missing something, this change is
nothing more than formatting that drops the use of an intermediate
variable. Please feel free to correct me if I am wrong here,
otherwise I believe the commit message should be changed to reflect
the code change.
You are missing the fact that ret getting set by a failing
xsm_getdomaininfo()
call might result in the ret value being propagated to the sysctl
caller. And
this should not happen. So the fix is to NOT modify ret here.
You are correct, my apologies for that.
No need to apologize. :-)
I believe it is proper to admit when you are wrong.
Second, as far as the problem description goes. The *only* time the
call to xsm_getdomaininfo() at this location will return anything
other than 0, is when FLASK is being used and a domain whose type is
not allowed getdomaininfo is making the call. XSM_HOOK signals a
no-op check for the default/dummy policy, and the SILO policy does
not override the default/dummy policy for this check.
Your statement sounds as if xsm_getdomaininfo() would always return
the same
value for a given caller domain. Isn't that return value also
depending on the
domain specified via the second parameter? In case it isn't, why does
that
parameter even exist?
It would if the default action was something other than XSM_HOOK. Look
at line 82 of include/xsm/dummy.h. XSM_HOOK will always return 0
regardless of the src or dest domains. The function
xsm_defualt_action() is the policy for both default/dummy and SILO
with the exception for evntchn, grants, and argo checks for SILO.
Ah, okay. I didn't analyze all of the involved xsm code.
No worries! I am always willing to help in any way that I can. While I
don't have the bandwidth to be proactive and keep up with everything on
xen-devel, please do not hesitate to ask me or ping me on anything XSM
related. I will gladly take a look and provide what insights I might
have on your query.
v/r,
dps
