sure, it wasn't mine! it was just missing one unveil so it's ok mestre@ on this one
On 11:42 Sat 15 Jun , Theo de Raadt wrote: > yeah that was my idea.. > > Ricardo Mestre <[email protected]> wrote: > > > Hi, > > > > Sorry to be late in the game, but as jca@ pointed out sysctl(8) tries to > > open _PATH_DEVDB first and then /dev if it cannot open the former, so > > both should be unveil(2)ed. Scramble the includes while at it. > > > > Index: sysctl.c > > =================================================================== > > RCS file: /cvs/src/sbin/sysctl/sysctl.c,v > > retrieving revision 1.242 > > diff -u -p -u -r1.242 sysctl.c > > --- sysctl.c 13 May 2019 20:47:19 -0000 1.242 > > +++ sysctl.c 14 Jun 2019 19:04:01 -0000 > > @@ -94,13 +94,14 @@ > > #include <ddb/db_var.h> > > #include <dev/rndvar.h> > > > > +#include <ctype.h> > > #include <err.h> > > #include <errno.h> > > +#include <limits.h> > > +#include <paths.h> > > #include <stdio.h> > > #include <stdlib.h> > > #include <string.h> > > -#include <ctype.h> > > -#include <limits.h> > > #include <unistd.h> > > > > #include <machine/cpu.h> > > @@ -162,6 +163,8 @@ struct list secondlevel[] = { > > > > int Aflag, aflag, nflag, qflag; > > > > +time_t boottime; > > + > > /* > > * Variables requiring special processing. > > */ > > @@ -255,6 +258,15 @@ main(int argc, char *argv[]) > > argc -= optind; > > argv += optind; > > > > + ctime(&boottime); /* satisfy potential $TZ expansion before unveil() */ > > + > > + if (unveil(_PATH_DEVDB, "r") == -1) > > + err(1,"unveil"); > > + if (unveil("/dev", "r") == -1) > > + err(1, "unveil"); > > + if (unveil(NULL, NULL) == -1) > > + err(1, "unveil"); > > + > > if (argc == 0 || (Aflag || aflag)) { > > debuginit(); > > vfsinit(); > > @@ -893,7 +905,6 @@ parse(char *string, int flags) > > } > > if (special & BOOTTIME) { > > struct timeval *btp = (struct timeval *)buf; > > - time_t boottime; > > > > if (!nflag) { > > boottime = btp->tv_sec; > > > > On 10:35 Sat 08 Jun , Theo de Raadt wrote: > > > When userland was massaged for pledge(), I hesitated using the > > > "manually call tzset()" approach for handling things. It felt > > > too low-level to call tzset(), an API almost noone knows the > > > existance of. > > > > > > Arriving in the same situation to satisfy unveil(). Again calling > > > tzset() feels too unfamiliar and low level. > > > > > > Regarding the comment in your diff, it says "localtime", but what is > > > actually called is ctime(), which calls localtime() (which calls > > > tzset(), which is where the unveil-files-missing or pledge-whatver > > > issues would show up in some programs). Probably should adjust > > > the comment > > > > > > Here's the later troublesome chunk: > > > > > > if (special & BOOTTIME) { > > > struct timeval *btp = (struct timeval *)buf; > > > time_t boottime; > > > > > > if (!nflag) { > > > boottime = btp->tv_sec; > > > (void)printf("%s%s%s", string, equ, > > > ctime(&boottime)); > > > > > > That makes me wonder, can we be less obtuse up front, and > > > prime the subsystem before unveil by calling the same function which > > > will be called later? > > > > > > Something like this. It feels slightly better to me. > > > > > > Index: sysctl.c > > > =================================================================== > > > RCS file: /cvs/src/sbin/sysctl/sysctl.c,v > > > retrieving revision 1.242 > > > diff -u -p -u -r1.242 sysctl.c > > > --- sysctl.c 13 May 2019 20:47:19 -0000 1.242 > > > +++ sysctl.c 8 Jun 2019 16:33:07 -0000 > > > @@ -162,6 +162,8 @@ struct list secondlevel[] = { > > > > > > int Aflag, aflag, nflag, qflag; > > > > > > +time_t boottime; > > > + > > > /* > > > * Variables requiring special processing. > > > */ > > > @@ -255,6 +257,12 @@ main(int argc, char *argv[]) > > > argc -= optind; > > > argv += optind; > > > > > > + ctime(&boottime); /* satisfy potential $TZ expansion before unveil() */ > > > + > > > + if (unveil("/dev", "r") == -1) > > > + err(1, "unveil"); > > > + if (unveil(NULL, NULL) == -1) > > > + err(1, "unveil"); > > > if (argc == 0 || (Aflag || aflag)) { > > > debuginit(); > > > vfsinit(); > > > @@ -893,7 +901,6 @@ parse(char *string, int flags) > > > } > > > if (special & BOOTTIME) { > > > struct timeval *btp = (struct timeval *)buf; > > > - time_t boottime; > > > > > > if (!nflag) { > > > boottime = btp->tv_sec; > > > > > > > > > > > > > > > > > > Florian Obser <[email protected]> wrote: > > > > > > > On Fri, Jun 07, 2019 at 11:24:30PM +0100, Ricardo Mestre wrote: > > > > > i did that and for some for reason i didn't get it! it tries to open > > > > > timezone so it kinda looks like a red flag right there... > > > > > > > > > > apart from /dev do we need to look into TZ on this one as well? if TZ > > > > > var needs to be looked at then all bets are off :/ > > > > > > > > this seems to do the right thing: > > > > > > > > diff --git sysctl.c sysctl.c > > > > index dc6abc16670..c74e706942a 100644 > > > > --- sysctl.c > > > > +++ sysctl.c > > > > @@ -255,6 +255,13 @@ main(int argc, char *argv[]) > > > > argc -= optind; > > > > argv += optind; > > > > > > > > + tzset(); /* for kern.boottime in localtime */ > > > > + > > > > + if (unveil("/dev", "r") == -1) > > > > + err(1, "unveil"); > > > > + if (unveil(NULL, NULL) == -1) > > > > + err(1, "unveil"); > > > > + > > > > if (argc == 0 || (Aflag || aflag)) { > > > > debuginit(); > > > > vfsinit(); > > > > > > > > > > > > -- > > > > I'm not entirely sure you are real. > > > > > > > > On 10:35 Sat 08 Jun , Theo de Raadt wrote: > > > When userland was massaged for pledge(), I hesitated using the > > > "manually call tzset()" approach for handling things. It felt > > > too low-level to call tzset(), an API almost noone knows the > > > existance of. > > > > > > Arriving in the same situation to satisfy unveil(). Again calling > > > tzset() feels too unfamiliar and low level. > > > > > > Regarding the comment in your diff, it says "localtime", but what is > > > actually called is ctime(), which calls localtime() (which calls > > > tzset(), which is where the unveil-files-missing or pledge-whatver > > > issues would show up in some programs). Probably should adjust > > > the comment > > > > > > Here's the later troublesome chunk: > > > > > > if (special & BOOTTIME) { > > > struct timeval *btp = (struct timeval *)buf; > > > time_t boottime; > > > > > > if (!nflag) { > > > boottime = btp->tv_sec; > > > (void)printf("%s%s%s", string, equ, > > > ctime(&boottime)); > > > > > > That makes me wonder, can we be less obtuse up front, and > > > prime the subsystem before unveil by calling the same function which > > > will be called later? > > > > > > Something like this. It feels slightly better to me. > > > > > > Index: sysctl.c > > > =================================================================== > > > RCS file: /cvs/src/sbin/sysctl/sysctl.c,v > > > retrieving revision 1.242 > > > diff -u -p -u -r1.242 sysctl.c > > > --- sysctl.c 13 May 2019 20:47:19 -0000 1.242 > > > +++ sysctl.c 8 Jun 2019 16:33:07 -0000 > > > @@ -162,6 +162,8 @@ struct list secondlevel[] = { > > > > > > int Aflag, aflag, nflag, qflag; > > > > > > +time_t boottime; > > > + > > > /* > > > * Variables requiring special processing. > > > */ > > > @@ -255,6 +257,12 @@ main(int argc, char *argv[]) > > > argc -= optind; > > > argv += optind; > > > > > > + ctime(&boottime); /* satisfy potential $TZ expansion before unveil() */ > > > + > > > + if (unveil("/dev", "r") == -1) > > > + err(1, "unveil"); > > > + if (unveil(NULL, NULL) == -1) > > > + err(1, "unveil"); > > > if (argc == 0 || (Aflag || aflag)) { > > > debuginit(); > > > vfsinit(); > > > @@ -893,7 +901,6 @@ parse(char *string, int flags) > > > } > > > if (special & BOOTTIME) { > > > struct timeval *btp = (struct timeval *)buf; > > > - time_t boottime; > > > > > > if (!nflag) { > > > boottime = btp->tv_sec; > > > > > > > > > > > > > > > > > > Florian Obser <[email protected]> wrote: > > > > > > > On Fri, Jun 07, 2019 at 11:24:30PM +0100, Ricardo Mestre wrote: > > > > > i did that and for some for reason i didn't get it! it tries to open > > > > > timezone so it kinda looks like a red flag right there... > > > > > > > > > > apart from /dev do we need to look into TZ on this one as well? if TZ > > > > > var needs to be looked at then all bets are off :/ > > > > > > > > this seems to do the right thing: > > > > > > > > diff --git sysctl.c sysctl.c > > > > index dc6abc16670..c74e706942a 100644 > > > > --- sysctl.c > > > > +++ sysctl.c > > > > @@ -255,6 +255,13 @@ main(int argc, char *argv[]) > > > > argc -= optind; > > > > argv += optind; > > > > > > > > + tzset(); /* for kern.boottime in localtime */ > > > > + > > > > + if (unveil("/dev", "r") == -1) > > > > + err(1, "unveil"); > > > > + if (unveil(NULL, NULL) == -1) > > > > + err(1, "unveil"); > > > > + > > > > if (argc == 0 || (Aflag || aflag)) { > > > > debuginit(); > > > > vfsinit(); > > > > > > > > > > > > -- > > > > I'm not entirely sure you are real. > > > >
