[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] sysctl: XSM hook should not cause XEN_SYSCTL_getdomaininfolist to (appear to) fail


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Date: Tue, 2 May 2023 06:43:33 -0400
  • Arc-authentication-results: i=1; mx.zohomail.com; dkim=pass header.i=apertussolutions.com; spf=pass smtp.mailfrom=dpsmith@xxxxxxxxxxxxxxxxxxxx; dmarc=pass header.from=<dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1683024218; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=oIghSZgYKIpdj1Zdp8vPzSF0OJ7ZJeiI5lUW7UH9DMc=; b=lEJDv+8p7ANbe+jHXLr9ek3lJJnbwo7A9KixtdajhGOiHQkG69e6v3ErvFxRJItAVBEoMc2DVsr0BD9DIgF6m8rIVM2Nc8mZjLmtP9WrQa5UXHnyljXbe7o8cImgG882xOBTUVRQ70mD80kLH7XcWzMxpG727SRTbpsaoUTxmn0=
  • Arc-seal: i=1; a=rsa-sha256; t=1683024218; cv=none; d=zohomail.com; s=zohoarc; b=la7k4xNjsSQnpYvPdTmYmMSusIMwiClw5LVgl55eUx8jePMgS0+ImyKRzXcJEfxnp4Qsbtohu8C/eSCAbe3h3CGJVn8JV+33RceH8R6+7v57jg4bCN5Lawl5FH6JHPZQa4ScMKt8Iejj8Dq0zTHdehDCbKk30DiwayZMblZGGhY=
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Tue, 02 May 2023 10:43:54 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>



On 5/2/23 03:17, Jan Beulich wrote:
Unlike for XEN_DOMCTL_getdomaininfo, where the XSM check is intended to
cause the operation to fail, in the loop here it ought to merely
determine whether information for the domain at hand may be reported
back. Therefore if on the last iteration the hook results in denial,
this should not affect the sub-op's return value.

Fixes: d046f361dc93 ("Xen Security Modules: XSM")
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
The hook being able to deny access to data for certain domains means
that no caller can assume to have a system-wide picture when holding the
results.

Wouldn't it make sense to permit the function to merely "count" domains?
While racy in general (including in its present, "normal" mode of
operation), within a tool stack this could be used as long as creation
of new domains is suppressed between obtaining the count and then using
it.

In XEN_DOMCTL_getpageframeinfo2 said commit had introduced a 2nd such
issue, but luckily that sub-op and xsm_getpageframeinfo() are long gone.


I understand there is a larger issue at play here but neutering the security control/XSM check is not the answer. This literally changes the way a FLASK policy that people currently have would be enforced, as well as contrary to how they understand the access control that it provides. Even though the code path does not fall under XSM maintainer, I would NACK this patch. IMHO, it is better to find a solution that does not abuse, misuse, or invalidate the purpose of the XSM calls.

On a side note, I am a little concern that only one person thought to include the XSM maintainer, or any of the XSM reviewers, onto a patch and the discussion around a patch that clearly relates to XSM for us to gauge the consequences of the patch. I am not assuming intentions here, only wanting to raise the concern.

So for what it is worth, NACK.

V/r,
Daniel P. Smith



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.