On Tue, Aug 06, 2019 at 06:57:49AM +0200, Sebastien Marie wrote:
> On Mon, Aug 05, 2019 at 07:21:22PM +0200, Alexander Bluhm wrote:
> > unveil(2) allocates 1024 bytes on the stack. That is a lot. Better
> > use namei pool like sys___realpath() does.
>
> There is a missing pool_put() in early return.
>
> 999 #ifdef KTRACE
> 1000 if (KTRPOINT(p, KTR_STRUCT))
> 1001 ktrstruct(p, "unveil", permissions,
> strlen(permissions));
> 1002 #endif
> 1003 if (pathlen < 2)
> 1004 return EINVAL;
>
> else it seems fine.
>
> ok semarie@ with that added.
oops, how could I miss that?
Perhaps a goto end is nicer then. sys___realpath does that also.
ok?
bluhm
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.332
diff -u -p -r1.332 vfs_syscalls.c
--- kern/vfs_syscalls.c 5 Aug 2019 23:28:55 -0000 1.332
+++ kern/vfs_syscalls.c 6 Aug 2019 19:47:29 -0000
@@ -975,7 +975,7 @@ sys_unveil(struct proc *p, void *v, regi
syscallarg(const char *) path;
syscallarg(const char *) permissions;
} */ *uap = v;
- char pathname[MAXPATHLEN], *c;
+ char *pathname, *c;
struct nameidata nd;
size_t pathlen;
char permissions[5];
@@ -992,17 +992,20 @@ 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);
+ return (error);
+ pathname = pool_get(&namei_pool, PR_WAITOK);
+ error = copyinstr(SCARG(uap, path), pathname, MAXPATHLEN, &pathlen);
if (error)
- return(error);
+ goto end;
#ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
ktrstruct(p, "unveil", permissions, strlen(permissions));
#endif
- if (pathlen < 2)
- return EINVAL;
+ if (pathlen < 2) {
+ error = EINVAL;
+ goto end;
+ }
/* find root "/" or "//" */
for (c = pathname; *c != '\0'; c++) {
@@ -1019,7 +1022,7 @@ sys_unveil(struct proc *p, void *v, regi
nd.ni_pledge = PLEDGE_UNVEIL;
if ((error = namei(&nd)) != 0)
- goto end;
+ goto ndfree;
/*
* XXX Any access to the file or directory will allow us to
@@ -1059,8 +1062,10 @@ sys_unveil(struct proc *p, void *v, regi
vrele(nd.ni_dvp);
pool_put(&namei_pool, nd.ni_cnd.cn_pnbuf);
-end:
+ndfree:
unveil_free_traversed_vnodes(&nd);
+end:
+ pool_put(&namei_pool, pathname);
return (error);
}
