I agree.

I'm going to write and test a similar diff for coredump().  It is called
at a deeper frame offset, thought the path is somewhat less arbitary and
user-controlled.

Alexander Bluhm <[email protected]> wrote:

> unveil(2) allocates 1024 bytes on the stack.  That is a lot.  Better
> use namei pool like sys___realpath() does.
>
> Index: kern/vfs_syscalls.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.331
> diff -u -p -r1.331 vfs_syscalls.c
> --- kern/vfs_syscalls.c       5 Aug 2019 15:13:43 -0000       1.331
> +++ kern/vfs_syscalls.c       5 Aug 2019 16:41:50 -0000
> @@ -969,7 +969,7 @@ sys_unveil(struct proc *p, void *v, regi
>               syscallarg(const char *) path;
>               syscallarg(const char *) permissions;
>       } */ *uap = v;
> -     char pathname[MAXPATHLEN];
> +     char *pathname;
>       struct nameidata nd;
>       size_t pathlen;
>       char permissions[5];
> @@ -986,10 +986,13 @@ sys_unveil(struct proc *p, void *v, regi
>       error = copyinstr(SCARG(uap, permissions), permissions,
>           sizeof(permissions), NULL);
>       if (error)
> -             return(error);
> -     error = copyinstr(SCARG(uap, path), pathname, sizeof(pathname), 
> &pathlen);
> -     if (error)
> -             return(error);
> +             return (error);
> +     pathname = pool_get(&namei_pool, PR_WAITOK);
> +     error = copyinstr(SCARG(uap, path), pathname, MAXPATHLEN, &pathlen);
> +     if (error) {
> +             pool_put(&namei_pool, pathname);
> +             return (error);
> +     }
> 
>  #ifdef KTRACE
>       if (KTRPOINT(p, KTR_STRUCT))
> @@ -1049,6 +1052,7 @@ sys_unveil(struct proc *p, void *v, regi
>       pool_put(&namei_pool, nd.ni_cnd.cn_pnbuf);
>  end:
>       unveil_free_traversed_vnodes(&nd);
> +     pool_put(&namei_pool, pathname);
> 
>       return (error);
>  }
> 

Reply via email to