From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH 1/2] xen/xsm: forbid PV guest console reads Date: Mon, 30 Sep 2013 13:29:00 -0400 Message-ID: <5249B4DC.8040208@tycho.nsa.gov> References: <1380556122-14067-1-git-send-email-dgdegra@tycho.nsa.gov> <5249BE7802000078000F824F@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VQhHV-0007bs-AY for xen-devel@lists.xenproject.org; Mon, 30 Sep 2013 17:29:13 +0000 In-Reply-To: <5249BE7802000078000F824F@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, keir@xen.org, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org On 09/30/2013 12:10 PM, Jan Beulich wrote: >>>> On 30.09.13 at 17:48, Daniel De Graaf wrote: >> When the hypervisor was compiled in debug mode (with VERBOSE defined), >> PV guests incorrectly had access to both read and write to the console. >> Change this to only allow write access; since such writes were limited >> by log levels in 48d50de8e0, remove the dependency on VERBOSE >> completely. > > I disagree, and iirc I disagreed already when you tried to drop the > dependency on VERBOSE with that earlier patch. > >> Reported-by: Jan Beulich >> Signed-off-by: Daniel De Graaf >> --- >> >> Alternatively, if controlling writes with VERBOSE is still desired, the >> ifdef VERBOSE can be retained surrounding the if() with the following >> commit message: >> >> > > That's what I'd want to see go in. > > Jan This patch retains the existing behavior where only HVM guests can use the console for output, and only via the 0xE9 I/O port. With Konrad's Linux patch, this means that xen_raw_console_write in non-dom0 will produce output only on Xen <= 4.3 (which returns -ENOSYS rather than -EPERM, as this code does). ------------------------8<---------------------------------------------- The CONSOLEIO_read operation was incorrectly allowed to PV guests if the hypervisor was compiled in debug mode (with VERBOSE defined). Reported-by: Jan Beulich Signed-off-by: Daniel De Graaf --- xen/include/xsm/dummy.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 2abf018..b1edd29 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -233,10 +233,10 @@ static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain *d, int cmd) { XSM_ASSERT_ACTION(XSM_OTHER); #ifdef VERBOSE - return xsm_default_action(XSM_HOOK, current->domain, NULL); -#else - return xsm_default_action(XSM_PRIV, current->domain, NULL); + if ( cmd == CONSOLEIO_write ) + return xsm_default_action(XSM_HOOK, d, NULL); #endif + return xsm_default_action(XSM_PRIV, d, NULL); } static XSM_INLINE int xsm_profile(XSM_DEFAULT_ARG struct domain *d, int op)