Re: [Minios-devel] [UNIKRAFT PATCH] lib/{ukmmap, vfscore}: fix null pointer dereferences

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Hugo,

I guess it is better to split the patch into a series of patches fixing 
it on mmap and vfscore separately.
Please find the review comments inline.


Thanks & Regards
Sharan

On 3/1/20 10:29 AM, Hugo Lefeuvre wrote:
> mmap and futimesat allocate buffers via malloc and dereference
> returned pointers without NULL checking, causing crashes in OOM
> situations.
>
> Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>
> ---
>   lib/ukmmap/mmap.c  | 6 ++++++
>   lib/vfscore/main.c | 6 ++++++
>   2 files changed, 12 insertions(+)
>
> diff --git a/lib/ukmmap/mmap.c b/lib/ukmmap/mmap.c
> index 7cee8dc..39ecbef 100644
> --- a/lib/ukmmap/mmap.c
> +++ b/lib/ukmmap/mmap.c
> @@ -101,6 +101,12 @@ void *mmap(void *addr, size_t len, int prot,
>                 return (void *) -1;
>         }
>         new = uk_malloc(uk_alloc_get_default(), sizeof(struct mmap_addr));
> +
> +       if (!new) {
> +               uk_free(uk_alloc_get_default(), mem);
> +               errno = ENOMEM;
> +               return (void *) -1;
> +       }
>         new->begin = mem;
>         new->end = mem + len;
>         new->next = NULL;
> diff --git a/lib/vfscore/main.c b/lib/vfscore/main.c
> index 7a7a54c..371290e 100644
> --- a/lib/vfscore/main.c
> +++ b/lib/vfscore/main.c
> @@ -1803,6 +1803,12 @@ int futimesat(int dirfd, const char *pathname, const 
> struct timeval times[2])
>   
>   	/* build absolute path */
>         absolute_path = (char*)malloc(PATH_MAX);
> +       if (!absolute_path) {
> +               fdrop(fp);
> +               error = ENOMEM;
The posix definition[1,2] for the function does not include ENOMEM as a 
possible error code. Unfortunately the only error which seems a bit 
relevant here is EFAULT.
> +               goto out_errno;
> +       }
> +
>         strlcpy(absolute_path, fp->f_dentry->d_mount->m_path, PATH_MAX);
>         strlcat(absolute_path, fp->f_dentry->d_path, PATH_MAX);
>   
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

[1] https://pubs.opengroup.org/onlinepubs/009695399/functions/utimes.html

[2] http://man7.org/linux/man-pages/man2/utimensat.2.html


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel