On 02.05.2023 05:59, Stefano Stabellini wrote: > xen: fix broken XEN_SYSCTL_getdomaininfolist hypercall > > XEN_SYSCTL_getdomaininfolist doesn't actually update the guest > num_domains field, only its local copy. Fix that.
This isn't true, at least not always / unconditionally. "copyback" is what controls copying back of the entire struct, and in the success case this looks to be happening fine. Yet for the failure case it's unclear whether any copying back is actually intended. (If the op was to return merely the number of active domains, I think that ought to be restricted to max_domains == 0 and the handle also being a null one. I'm also having a hard time seeing what failure case the test ended up encountering: There are only two errors which can occur - one from the XSM hook (which is mishandled, and I'll make a separate patch for that) and the other from failing to copy back the info for the domain being looked at. I hope we can exclude the former, so are you suggesting the info struct copy-back is failing in your case? > Signed-off-by: Stefano Stabellini <[email protected]> In any event, if anything needs fixing here, a Fixes: tag would be nice. Jan > diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c > index 02505ab044..0e1097be96 100644 > --- a/xen/common/sysctl.c > +++ b/xen/common/sysctl.c > @@ -107,10 +107,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) > u_sysctl) > > rcu_read_unlock(&domlist_read_lock); > > - if ( ret != 0 ) > - break; > - > op->u.getdomaininfolist.num_domains = num_domains; > + __copy_field_to_guest(u_sysctl, op, u.getdomaininfolist.num_domains); > } > break; >
