Re: [Xen-devel] [PATCH v2 11/18] mini-os: add checks for out of memory

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, on Fri 05 Aug 2016 19:35:55 +0200, wrote:
> There are several core functions in Mini-OS not checking for failed
> memory allocations. Add such checks.
> 
> Add do_map_frames() dummy function to arm architecture as it will be
> needed in future for compilations to succeed.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

> ---
>  arch/arm/mm.c |  8 ++++++++
>  arch/x86/mm.c | 26 +++++++++++++++++++-------
>  include/mm.h  |  2 +-
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mm.c b/arch/arm/mm.c
> index f75888d..fc8d4bc 100644
> --- a/arch/arm/mm.c
> +++ b/arch/arm/mm.c
> @@ -1,6 +1,7 @@
>  #include <mini-os/console.h>
>  #include <xen/memory.h>
>  #include <arch_mm.h>
> +#include <mini-os/errno.h>
>  #include <mini-os/hypervisor.h>
>  #include <libfdt.h>
>  #include <lib.h>
> @@ -79,6 +80,13 @@ void arch_init_demand_mapping_area(void)
>  {
>  }
>  
> +int do_map_frames(unsigned long addr,
> +        const unsigned long *f, unsigned long n, unsigned long stride,
> +        unsigned long increment, domid_t id, int *err, unsigned long prot)
> +{
> +    return -ENOSYS;
> +}
> +
>  /* Get Xen's suggested physical page assignments for the grant table. */
>  static paddr_t get_gnttab_base(void)
>  {
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index e2f026b..12f7fe4 100644
> --- a/arch/x86/mm.c
> +++ b/arch/x86/mm.c
> @@ -34,6 +34,7 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
>  
> +#include <mini-os/errno.h>
>  #include <mini-os/os.h>
>  #include <mini-os/hypervisor.h>
>  #include <mini-os/mm.h>
> @@ -354,6 +355,8 @@ pgentry_t *need_pgt(unsigned long va)
>      if ( !(tab[offset] & _PAGE_PRESENT) )
>      {
>          pt_pfn = virt_to_pfn(alloc_page());
> +        if ( !pt_pfn )
> +            return NULL;
>          new_pt_frame(&pt_pfn, pt_mfn, offset, L3_FRAME);
>      }
>      ASSERT(tab[offset] & _PAGE_PRESENT);
> @@ -364,6 +367,8 @@ pgentry_t *need_pgt(unsigned long va)
>      if ( !(tab[offset] & _PAGE_PRESENT) ) 
>      {
>          pt_pfn = virt_to_pfn(alloc_page());
> +        if ( !pt_pfn )
> +            return NULL;
>          new_pt_frame(&pt_pfn, pt_mfn, offset, L2_FRAME);
>      }
>      ASSERT(tab[offset] & _PAGE_PRESENT);
> @@ -373,6 +378,8 @@ pgentry_t *need_pgt(unsigned long va)
>      if ( !(tab[offset] & _PAGE_PRESENT) )
>      {
>          pt_pfn = virt_to_pfn(alloc_page());
> +        if ( !pt_pfn )
> +            return NULL;
>          new_pt_frame(&pt_pfn, pt_mfn, offset, L1_FRAME);
>      }
>      ASSERT(tab[offset] & _PAGE_PRESENT);
> @@ -445,10 +452,10 @@ unsigned long allocate_ondemand(unsigned long n, 
> unsigned long alignment)
>   * va. map f[i*stride]+i*increment for i in 0..n-1.
>   */
>  #define MAP_BATCH ((STACK_SIZE / 2) / sizeof(mmu_update_t))
> -void do_map_frames(unsigned long va,
> -                   const unsigned long *mfns, unsigned long n, 
> -                   unsigned long stride, unsigned long incr, 
> -                   domid_t id, int *err, unsigned long prot)
> +int do_map_frames(unsigned long va,
> +                  const unsigned long *mfns, unsigned long n,
> +                  unsigned long stride, unsigned long incr,
> +                  domid_t id, int *err, unsigned long prot)
>  {
>      pgentry_t *pgt = NULL;
>      unsigned long done = 0;
> @@ -458,7 +465,7 @@ void do_map_frames(unsigned long va,
>      if ( !mfns ) 
>      {
>          printk("do_map_frames: no mfns supplied\n");
> -        return;
> +        return -EINVAL;
>      }
>      DEBUG("va=%p n=0x%lx, mfns[0]=0x%lx stride=0x%lx incr=0x%lx 
> prot=0x%lx\n",
>            va, n, mfns[0], stride, incr, prot);
> @@ -484,7 +491,9 @@ void do_map_frames(unsigned long va,
>              {
>                  if ( !pgt || !(va & L1_MASK) )
>                      pgt = need_pgt(va);
> -                
> +                if ( !pgt )
> +                    return -ENOMEM;
> +
>                  mmu_updates[i].ptr = virt_to_mach(pgt) | 
> MMU_NORMAL_PT_UPDATE;
>                  mmu_updates[i].val = ((pgentry_t)(mfns[(done + i) * stride] +
>                                                    (done + i) * incr)
> @@ -505,6 +514,8 @@ void do_map_frames(unsigned long va,
>          }
>          done += todo;
>      }
> +
> +    return 0;
>  }
>  
>  /*
> @@ -521,7 +532,8 @@ void *map_frames_ex(const unsigned long *mfns, unsigned 
> long n,
>      if ( !va )
>          return NULL;
>  
> -    do_map_frames(va, mfns, n, stride, incr, id, err, prot);
> +    if ( do_map_frames(va, mfns, n, stride, incr, id, err, prot) )
> +        return NULL;
>  
>      return (void *)va;
>  }
> diff --git a/include/mm.h b/include/mm.h
> index a22dcd1..9244e26 100644
> --- a/include/mm.h
> +++ b/include/mm.h
> @@ -68,7 +68,7 @@ unsigned long allocate_ondemand(unsigned long n, unsigned 
> long alignment);
>  void *map_frames_ex(const unsigned long *f, unsigned long n, unsigned long 
> stride,
>       unsigned long increment, unsigned long alignment, domid_t id,
>       int *err, unsigned long prot);
> -void do_map_frames(unsigned long addr,
> +int do_map_frames(unsigned long addr,
>          const unsigned long *f, unsigned long n, unsigned long stride,
>       unsigned long increment, domid_t id, int *err, unsigned long prot);
>  int unmap_frames(unsigned long va, unsigned long num_frames);
> -- 
> 2.6.6
> 

-- 
Samuel
"How should I know if it works?  That's what beta testers are for.  I only
coded it."
(Attributed to Linus Torvalds, somewhere in a posting)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel