make domain_create() return a proper error code While triggered by the XSA-9 fix, this really is of more general use; that fix just pointed out very sharply that the current situation with all domain creation failures reported to user (tools) space as -ENOMEM is very unfortunate (actively misleading users _and_ support personnel). Pull over the pointer <-> error code conversion infrastructure from Linux, and use it in domain_create() and all it callers. Signed-off-by: Jan Beulich --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -237,7 +238,7 @@ void __init start_xen(unsigned long boot /* Create initial domain 0. */ dom0 = domain_create(0, 0, 0); - if ( dom0 == NULL ) + if ( IS_ERR(dom0) ) printk("domain_create failed\n"); if ( (dom0 == NULL) || (alloc_dom0_vcpu0() == NULL) ) panic("Error creating domain 0\n"); --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -91,7 +91,7 @@ #include #include #include -#include +#include #include #include #include @@ -309,7 +309,7 @@ void __init arch_init_memory(void) * their domain field set to dom_xen. */ dom_xen = domain_create(DOMID_XEN, DOMCRF_dummy, 0); - BUG_ON(dom_xen == NULL); + BUG_ON(IS_ERR(dom_xen)); /* * Initialise our DOMID_IO domain. @@ -317,14 +317,14 @@ void __init arch_init_memory(void) * array. Mappings occur at the priv of the caller. */ dom_io = domain_create(DOMID_IO, DOMCRF_dummy, 0); - BUG_ON(dom_io == NULL); + BUG_ON(IS_ERR(dom_io)); /* - * Initialise our DOMID_IO domain. + * Initialise our COW domain. * This domain owns sharable pages. */ dom_cow = domain_create(DOMID_COW, DOMCRF_dummy, 0); - BUG_ON(dom_cow == NULL); + BUG_ON(IS_ERR(dom_cow)); /* First 1MB of RAM is historically marked as I/O. */ for ( i = 0; i < 0x100; i++ ) --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -1319,7 +1320,7 @@ void __init __start_xen(unsigned long mb /* Create initial domain 0. */ dom0 = domain_create(0, DOMCRF_s3_integrity, 0); - if ( (dom0 == NULL) || (alloc_dom0_vcpu0() == NULL) ) + if ( IS_ERR(dom0) || (alloc_dom0_vcpu0() == NULL) ) panic("Error creating domain 0\n"); dom0->is_privileged = 1; --- a/xen/common/cpupool.c +++ b/xen/common/cpupool.c @@ -370,14 +370,18 @@ out: int cpupool_add_domain(struct domain *d, int poolid) { struct cpupool *c; - int rc = 1; + int rc; int n_dom = 0; if ( poolid == CPUPOOLID_NONE ) return 0; spin_lock(&cpupool_lock); c = cpupool_find_by_id(poolid); - if ( (c != NULL) && cpumask_weight(c->cpu_valid) ) + if ( c == NULL ) + rc = -ESRCH; + else if ( !cpumask_weight(c->cpu_valid) ) + rc = -ENODEV; + else { c->n_dom++; n_dom = c->n_dom; --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include @@ -196,17 +196,17 @@ struct domain *domain_create( struct domain *d, **pd; enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2, INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 }; - int init_status = 0; + int err, init_status = 0; int poolid = CPUPOOLID_NONE; if ( (d = alloc_domain_struct()) == NULL ) - return NULL; + return ERR_PTR(-ENOMEM); d->domain_id = domid; lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid, "Domain"); - if ( xsm_alloc_security_domain(d) != 0 ) + if ( (err = xsm_alloc_security_domain(d)) != 0 ) goto fail; init_status |= INIT_xsm; @@ -226,6 +226,7 @@ struct domain *domain_create( spin_lock_init(&d->shutdown_lock); d->shutdown_code = -1; + err = -ENOMEM; if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) ) goto fail; @@ -251,7 +252,7 @@ struct domain *domain_create( if ( !is_idle_domain(d) ) { - if ( xsm_domain_create(d, ssidref) != 0 ) + if ( (err = xsm_domain_create(d, ssidref)) != 0 ) goto fail; d->is_paused_by_controller = 1; @@ -266,29 +267,30 @@ struct domain *domain_create( radix_tree_init(&d->pirq_tree); - if ( evtchn_init(d) != 0 ) + if ( (err = evtchn_init(d)) != 0 ) goto fail; init_status |= INIT_evtchn; - if ( grant_table_create(d) != 0 ) + if ( (err = grant_table_create(d)) != 0 ) goto fail; init_status |= INIT_gnttab; poolid = 0; + err = -ENOMEM; d->mem_event = xzalloc(struct mem_event_per_domain); if ( !d->mem_event ) goto fail; } - if ( arch_domain_create(d, domcr_flags) != 0 ) + if ( (err = arch_domain_create(d, domcr_flags)) != 0 ) goto fail; init_status |= INIT_arch; - if ( cpupool_add_domain(d, poolid) != 0 ) + if ( (err = cpupool_add_domain(d, poolid)) != 0 ) goto fail; - if ( sched_init_domain(d) != 0 ) + if ( (err = sched_init_domain(d)) != 0 ) goto fail; if ( !is_idle_domain(d) ) @@ -329,7 +331,7 @@ struct domain *domain_create( xsm_free_security_domain(d); free_cpumask_var(d->domain_dirty_cpumask); free_domain_struct(d); - return NULL; + return ERR_PTR(err); } --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -455,10 +456,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off ) domcr_flags |= DOMCRF_oos_off; - ret = -ENOMEM; d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref); - if ( d == NULL ) + if ( IS_ERR(d) ) + { + ret = PTR_ERR(d); break; + } ret = 0; --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include @@ -1323,7 +1323,7 @@ void __init scheduler_init(void) panic("scheduler returned error on init\n"); idle_domain = domain_create(DOMID_IDLE, 0, 0); - BUG_ON(idle_domain == NULL); + BUG_ON(IS_ERR(idle_domain)); idle_domain->vcpu = idle_vcpu; idle_domain->max_vcpus = nr_cpu_ids; if ( alloc_vcpu(idle_domain, 0, 0) == NULL ) --- /dev/null +++ b/xen/include/xen/err.h @@ -0,0 +1,57 @@ +#if !defined(__XEN_ERR_H__) && !defined(__ASSEMBLY__) +#define __XEN_ERR_H__ + +#include +#include + +/* + * Kernel pointers have redundant information, so we can use a + * scheme where we can return either an error code or a dentry + * pointer with the same return value. + * + * This could be a per-architecture thing, to allow different + * error and pointer decisions. + */ +#define MAX_ERRNO 4095 + +#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO) + +static inline void *__must_check ERR_PTR(long error) +{ + return (void *)error; +} + +static inline long __must_check PTR_ERR(const void *ptr) +{ + return (long)ptr; +} + +static inline long __must_check IS_ERR(const void *ptr) +{ + return IS_ERR_VALUE((unsigned long)ptr); +} + +static inline long __must_check IS_ERR_OR_NULL(const void *ptr) +{ + return !ptr || IS_ERR_VALUE((unsigned long)ptr); +} + +/** + * ERR_CAST - Explicitly cast an error-valued pointer to another pointer type + * @ptr: The pointer to cast. + * + * Explicitly cast an error-valued pointer to another pointer type in such a + * way as to make it clear that's what's going on. + */ +static inline void * __must_check ERR_CAST(const void *ptr) +{ + /* cast away the const */ + return (void *)ptr; +} + +static inline int __must_check PTR_RET(const void *ptr) +{ + return IS_ERR(ptr) ? PTR_ERR(ptr) : 0; +} + +#endif /* __XEN_ERR_H__ */