On Thu, Oct 29, 2015 at 04:32:25PM +0000, Peter J. Philipp wrote:
> Hi deraadt,
>
> I know you know I don't code well, but in order to show you what's on my
> mind I had to write code, I took the bsearch() from the ieee80211 code, so
> perhaps there is a better way (like always) perhaps to unify the function
> between these two areas.
>
> The reason I did this is to save on cpu cycles when you look at X amount
> of processes all pledging...one time'd process won't show much difference.
>
I'm not deraadt, but -
smart but have you considered that this is not worth the effort?
pledge() is only called once or twice during a process' lifetime -
start, pledge, run - the linear search on such a short list is still
relatively fast, and the entries are even sorted in the order of
relevance. By convention "stdio" always comes first. So what is the
impact without your diff of pledge in src/bin/sleep/sleep.c:
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
$ time obj/sleep 0.01
0m00.01s real 0m00.00s user 0m00.01s system
Are you trying to solve an issue?
Reyk
> below's diff:
>
> -peter
>
>
> ? pledge.diff
> Index: kern/kern_pledge.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.90
> diff -u -p -u -r1.90 kern_pledge.c
> --- kern/kern_pledge.c 28 Oct 2015 17:38:52 -0000 1.90
> +++ kern/kern_pledge.c 29 Oct 2015 16:23:04 -0000
> @@ -58,6 +58,32 @@
>
> int canonpath(const char *input, char *buf, size_t bufsize);
> int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
> +int pledge_cmp(const void *pi, const void *ph);
> +
> +/* taken from net80211/ieee80211_regdomain.c */
> +const void *pledge_bsearch(const void *, const void *, size_t, size_t,
> + int (*)(const void *, const void *));
> +
> +const void *
> +pledge_bsearch(const void *key, const void *base0, size_t nmemb, size_t size,
> + int (*compar)(const void *, const void *))
> +{
> + const char *base = base0;
> + int lim, cmp;
> + const void *p;
> +
> + for (lim = nmemb; lim != 0; lim >>= 1) {
> + p = base + (lim >> 1) * size;
> + cmp = (*compar)(key, p);
> + if (cmp == 0)
> + return ((const void *)p);
> + if (cmp > 0) { /* key > p: move right */
> + base = (const char *)p + size;
> + lim--;
> + } /* else move left */
> + }
> + return (NULL);
> +}
>
> const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
> [SYS_exit] = 0xffffffff,
> @@ -256,40 +282,42 @@ const u_int pledge_syscalls[SYS_MAXSYSCA
> [SYS_swapctl] = PLEDGE_VMINFO, /* XXX should limit to "get" operations
> */
> };
>
> -static const struct {
> +/* MUST be sorted! */
> +static const struct PR {
> char *name;
> int flags;
> } pledgereq[] = {
> - { "stdio", PLEDGE_STDIO },
> - { "rpath", PLEDGE_RPATH },
> - { "wpath", PLEDGE_WPATH },
> - { "tmppath", PLEDGE_TMPPATH },
> - { "inet", PLEDGE_INET },
> - { "unix", PLEDGE_UNIX },
> + { "abort", 0 }, /* XXX reserve for later */
> + { "cpath", PLEDGE_CPATH },
> { "dns", PLEDGE_DNS },
> + { "exec", PLEDGE_EXEC },
> + { "fattr", PLEDGE_FATTR },
> + { "flock", PLEDGE_FLOCK },
> { "getpw", PLEDGE_GETPW },
> - { "sendfd", PLEDGE_SENDFD },
> - { "recvfd", PLEDGE_RECVFD },
> - { "ioctl", PLEDGE_IOCTL },
> { "id", PLEDGE_ID },
> - { "route", PLEDGE_ROUTE },
> + { "inet", PLEDGE_INET },
> + { "ioctl", PLEDGE_IOCTL },
> { "mcast", PLEDGE_MCAST },
> - { "tty", PLEDGE_TTY },
> { "proc", PLEDGE_PROC },
> - { "exec", PLEDGE_EXEC },
> - { "cpath", PLEDGE_CPATH },
> - { "fattr", PLEDGE_FATTR },
> { "prot_exec", PLEDGE_PROTEXEC },
> - { "flock", PLEDGE_FLOCK },
> { "ps", PLEDGE_PS },
> - { "vminfo", PLEDGE_VMINFO },
> + { "recvfd", PLEDGE_RECVFD },
> + { "route", PLEDGE_ROUTE },
> + { "rpath", PLEDGE_RPATH },
> + { "sendfd", PLEDGE_SENDFD },
> { "settime", PLEDGE_SETTIME },
> - { "abort", 0 }, /* XXX reserve for later */
> + { "stdio", PLEDGE_STDIO },
> + { "tmppath", PLEDGE_TMPPATH },
> + { "tty", PLEDGE_TTY },
> + { "unix", PLEDGE_UNIX },
> + { "vminfo", PLEDGE_VMINFO },
> + { "wpath", PLEDGE_WPATH },
> };
>
> int
> sys_pledge(struct proc *p, void *v, register_t *retval)
> {
> + struct PR *pr = NULL;
> struct sys_pledge_args /* {
> syscallarg(const char *)request;
> syscallarg(const char **)paths;
> @@ -300,7 +328,7 @@ sys_pledge(struct proc *p, void *v, regi
> if (SCARG(uap, request)) {
> size_t rbuflen;
> char *rbuf, *rp, *pn;
> - int f, i;
> + int f;
>
> rbuf = malloc(MAXPATHLEN, M_TEMP, M_WAITOK);
> error = copyinstr(SCARG(uap, request), rbuf, MAXPATHLEN,
> @@ -321,16 +349,15 @@ sys_pledge(struct proc *p, void *v, regi
> *pn++ = '\0';
> }
>
> - for (f = i = 0; i < nitems(pledgereq); i++) {
> - if (strcmp(rp, pledgereq[i].name) == 0) {
> - f = pledgereq[i].flags;
> - break;
> - }
> - }
> - if (f == 0) {
> - free(rbuf, M_TEMP, MAXPATHLEN);
> - return (EINVAL);
> + if ((pr = (struct PR *)pledge_bsearch(rp, &pledgereq,
> + nitems(pledgereq), sizeof(struct PR),
> + pledge_cmp)) == NULL) {
> + free(rbuf, M_TEMP, MAXPATHLEN);
> + return (EINVAL);
> }
> +
> + f = pr->flags;
> +
> flags |= f;
> }
> free(rbuf, M_TEMP, MAXPATHLEN);
> @@ -1377,4 +1404,10 @@ substrcmp(const char *p1, size_t s1, con
> return (2); /* string2 is a subpath of string1 */
> else
> return (0); /* no subpath */
> +}
> +
> +int
> +pledge_cmp(const void *pi, const void *ph)
> +{
> + return(strcmp((const char *)pi, ((const struct PR *)ph)->name));
> }
>
--
