On 10/29/15 18:51, Reyk Floeter wrote:
> 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");
Hi Reyk,
deraadt already told me there was a patch for this already. Yes it
would be more cycles for stdio I see that.
Thanks for your effort in making me see this.
-peter
> $ 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));
>> }
>>
