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.
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? Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
