From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH] Fix boot crash on xsm/flask enabled builds when no policy module is present Date: Mon, 26 Aug 2013 09:27:07 -0400 Message-ID: <521B57AB.9080007@tycho.nsa.gov> References: <1377511404-3365-1-git-send-email-tomasz.wroblewski@citrix.com> <521B335F.9020004@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VDwpD-00059S-CT for xen-devel@lists.xenproject.org; Mon, 26 Aug 2013 13:27:19 +0000 In-Reply-To: <521B335F.9020004@citrix.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: Andrew Cooper , Jan Beulich Cc: Tomasz Wroblewski , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 08/26/2013 06:52 AM, Andrew Cooper wrote: > On 26/08/2013 11:03, Tomasz Wroblewski wrote: >> Xen crashes on boot of xsm/flask enabled builds, if policy module is not specified. >> This seems to have worked on 4.1 at least. Can be fixed by testing whether policy_buffer >> is NULL before attempting to load from it - it's a global which is set to non-NULL when >> policy module is detected. >> >> Signed-off-by: Tomasz Wroblewski > > CCing Daniel De Graaf, as the maintainer of this code. > > However FWIW, > Reviewed-by: Andrew Cooper > >> --- >> xen/xsm/flask/hooks.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c >> index fa0589a..cfa2929 100644 >> --- a/xen/xsm/flask/hooks.c >> +++ b/xen/xsm/flask/hooks.c >> @@ -1585,7 +1585,8 @@ static __init int flask_init(void) >> if ( register_xsm(&flask_ops) ) >> panic("Flask: Unable to register with XSM.\n"); >> >> - ret = security_load_policy(policy_buffer, policy_size); >> + if ( policy_buffer ) >> + ret = security_load_policy(policy_buffer, policy_size); >> >> if ( flask_enforcing ) >> printk("Flask: Starting in enforcing mode.\n"); > While this change is not wrong, I also don't see how it could fix anything. The security_load_policy function will not dereference policy_buffer if policy_size is zero (it'll return -EINVAL first), and the only location that sets policy_size (xsm_policy_init) also sets policy_buffer. If this function is setting the NULL pointer, it also does a dereference - and it will be visible in the printk. Also, on 08/26/2013 07:12 AM, Jan Beulich wrote: > Question is whether policy_buffer == NULL really isn't supposed > to result in a -E... return value (as in fact flask initialization failed). The return value of flask_init isn't ever checked. A failure to load the policy at boot is identical to no policy - waiting for a flask_disable or "xl loadpolicy" hypercall from dom0. -- Daniel De Graaf National Security Agency