Re: [XenPPC] [RFC][PATCH] Isolating ACM's architecture-dependent parts

[Hollis Blanchard <hollisb@xxxxxxxxxx>]

On Tue, 2006-09-12 at 21:41 -0400, Stefan Berger wrote:
> 
> Index: root/xen-unstable.hg/xen/acm/Makefile
> ===================================================================
> --- root.orig/xen-unstable.hg/xen/acm/Makefile
> +++ root/xen-unstable.hg/xen/acm/Makefile
> @@ -3,3 +3,5 @@ obj-y += acm_policy.o
>  obj-y += acm_simple_type_enforcement_hooks.o
>  obj-y += acm_chinesewall_hooks.o
>  obj-y += acm_null_hooks.o
> +obj-$(x86_32) += acm_multiboot.o
> +obj-$(x86_64) += acm_multiboot.o 

config/x86_* defines CONFIG_X86, so that would be better to use here.

> Index: root/xen-unstable.hg/xen/include/asm-x86/acm.h
> ===================================================================
> --- /dev/null
> +++ root/xen-unstable.hg/xen/include/asm-x86/acm.h
> @@ -0,0 +1,91 @@
> +#ifndef _XEN_ASM_ACM_H
> +#define _XEN_ASM_ACM_H
> +
> +#include <xen/multiboot.h>
> +#include <acm/acm_hooks.h>
> +
> +#ifdef ACM_SECURITY
> +
> +/* Fetch acm policy module from multiboot modules.  */
> +static inline void
> +extract_acm_policy(multiboot_info_t *mbi,
> +                   unsigned int *initrdidx,
> +                   unsigned long initial_images_start,
> +                   char **_policy_start, unsigned long *_policy_len)
> +{
> +    int i;
> +    module_t *mod = (module_t *)__va(mbi->mods_addr);
> +
> +    if ( mbi->mods_count > 1 )
> +        *initrdidx = 1;
> +
> +    /*
> +     * Try all modules and see whichever could be the binary policy.
> +     * Adjust the initrdidx if module[1] is the binary policy.
> +     */
> +    for ( i = mbi->mods_count-1; i >= 1; i-- )
> +    {
> +        unsigned long start;
> +        char *policy_start;
> +        unsigned long policy_len;
> +
> +        start = initial_images_start +
> (mod[i].mod_start-mod[0].mod_start);
> +#if defined(__i386__)
> +        policy_start = (char *)start;
> +#elif defined(__x86_64__)
> +        policy_start = __va(start);
> +#endif
> +        policy_len   = mod[i].mod_end - mod[i].mod_start;
> +        if ( acm_is_policy(policy_start, policy_len) )
> +        {
> +            printf("Policy len  0x%lx, start at %p - module %d.\n",
> +                   policy_len, policy_start, i);
> +            *_policy_start = policy_start;
> +            *_policy_len = policy_len;
> +            if ( i == 1 )
> +            {
> +                if (mbi->mods_count > 2)
> +                    *initrdidx = 2;
> +                else
> +                    *initrdidx = 0;
> +            }
> +            else
> +                *initrdidx = 1;
> +            break;
> +        }
> +    }
> +}
> +
> +static inline
> +int acm_x86_init(multiboot_info_t *mbi,
> +                 unsigned int *initrdidx,
> +                 unsigned long initial_images_start)
> +{
> +    char *_policy_start = NULL;
> +    unsigned long _policy_len = 0;
> +    /* Extract policy from multiboot.  */
> +    extract_acm_policy(mbi,
> +                       initrdidx,
> +                       initial_images_start,
> +                       &_policy_start, &_policy_len);
> +
> +    /*
> +     * Initialize access control security module no matter whether
> +     * a policy has been found or not.
> +     */
> +    return acm_init(_policy_start, _policy_len);
> +}
> +
> +#else
> +
> +static inline
> +int acm_x86_init(multiboot_info_t *mbi,
> +                 unsigned int *initrdidx,
> +                 unsigned long initial_images_start)
> +{
> +    return 0;
> +}
> +
> +#endif
> +
> +#endif

These are way too big to be static inlines. Make them regular functions
please.

Other than that, this patch seems great.

-- 
Hollis Blanchard
IBM Linux Technology Center


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel