Re: disable libc sys wrappers?

2020-07-14 Thread Paul Irofti

On 13.07.2020 20:43, Ted Unangst wrote:

On 2020-07-09, Theo de Raadt wrote:

Added a -T option to ktrace for transparency. I got ambitious here and made
it take suboptions, anticipating that other transparency modifications may
be desired.


Please don't do that.


Here is a simpler version.


OK.




Index: lib/libc/dlfcn/init.c
===
RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
retrieving revision 1.8
diff -u -p -r1.8 init.c
--- lib/libc/dlfcn/init.c   6 Jul 2020 13:33:05 -   1.8
+++ lib/libc/dlfcn/init.c   13 Jul 2020 17:36:04 -
@@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
_timekeep->tk_version != TK_VERSION)
_timekeep = NULL;
}
+   if (issetugid() == 0 && getenv("LIBC_NOUSERTC"))
+   _timekeep = NULL;
break;
}
}
Index: usr.bin/ktrace/ktrace.1
===
RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.1,v
retrieving revision 1.30
diff -u -p -r1.30 ktrace.1
--- usr.bin/ktrace/ktrace.1 15 May 2019 15:36:59 -  1.30
+++ usr.bin/ktrace/ktrace.1 13 Jul 2020 17:38:22 -
@@ -37,13 +37,13 @@
  .Nd enable kernel process tracing
  .Sh SYNOPSIS
  .Nm ktrace
-.Op Fl aBCcdi
+.Op Fl aCcdi
  .Op Fl f Ar trfile
  .Op Fl g Ar pgid
  .Op Fl p Ar pid
  .Op Fl t Ar trstr
  .Nm ktrace
-.Op Fl adi
+.Op Fl aBdiT
  .Op Fl f Ar trfile
  .Op Fl t Ar trstr
  .Ar command
@@ -109,6 +109,8 @@ processes.
  Enable (disable) tracing on the indicated process ID (only one
  .Fl p
  flag is permitted).
+.It Fl T
+Disable userland timekeeping, making time related system calls more prevalent.
  .It Fl t Ar trstr
  Select which information to put into the dump file.
  The argument can contain one or more of the following letters.
Index: usr.bin/ktrace/ktrace.c
===
RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.c,v
retrieving revision 1.36
diff -u -p -r1.36 ktrace.c
--- usr.bin/ktrace/ktrace.c 28 Jun 2019 13:35:01 -  1.36
+++ usr.bin/ktrace/ktrace.c 13 Jul 2020 17:37:06 -
@@ -100,7 +100,7 @@ main(int argc, char *argv[])
usage();
}
} else {
-   while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:")) != -1)
+   while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:T")) != -1)
switch ((char)ch) {
case 'a':
append = 1;
@@ -140,6 +140,9 @@ main(int argc, char *argv[])
usage();
}
break;
+   case 'T':
+   putenv("LIBC_NOUSERTC=");
+   break;
default:
usage();
}
@@ -240,9 +243,9 @@ usage(void)
" [-u trspec] command\n",
__progname);
else
-   fprintf(stderr, "usage: %s [-aBCcdi] [-f trfile] [-g pgid]"
+   fprintf(stderr, "usage: %s [-aCcdi] [-f trfile] [-g pgid]"
" [-p pid] [-t trstr]\n"
-   "   %s [-adi] [-f trfile] [-t trstr] command\n",
+   "   %s [-aBdiT] [-f trfile] [-t trstr] command\n",
__progname, __progname);
exit(1);
  }





Re: disable libc sys wrappers?

2020-07-13 Thread Theo de Raadt
Fine with me.


Ted Unangst  wrote:

> On 2020-07-09, Theo de Raadt wrote:
> > > Added a -T option to ktrace for transparency. I got ambitious here and 
> > > made
> > > it take suboptions, anticipating that other transparency modifications may
> > > be desired.
> > 
> > Please don't do that.
> 
> Here is a simpler version.
> 
> 
> Index: lib/libc/dlfcn/init.c
> ===
> RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 init.c
> --- lib/libc/dlfcn/init.c 6 Jul 2020 13:33:05 -   1.8
> +++ lib/libc/dlfcn/init.c 13 Jul 2020 17:36:04 -
> @@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
>   _timekeep->tk_version != TK_VERSION)
>   _timekeep = NULL;
>   }
> + if (issetugid() == 0 && getenv("LIBC_NOUSERTC"))
> + _timekeep = NULL;
>   break;
>   }
>   }
> Index: usr.bin/ktrace/ktrace.1
> ===
> RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.1,v
> retrieving revision 1.30
> diff -u -p -r1.30 ktrace.1
> --- usr.bin/ktrace/ktrace.1   15 May 2019 15:36:59 -  1.30
> +++ usr.bin/ktrace/ktrace.1   13 Jul 2020 17:38:22 -
> @@ -37,13 +37,13 @@
>  .Nd enable kernel process tracing
>  .Sh SYNOPSIS
>  .Nm ktrace
> -.Op Fl aBCcdi
> +.Op Fl aCcdi
>  .Op Fl f Ar trfile
>  .Op Fl g Ar pgid
>  .Op Fl p Ar pid
>  .Op Fl t Ar trstr
>  .Nm ktrace
> -.Op Fl adi
> +.Op Fl aBdiT
>  .Op Fl f Ar trfile
>  .Op Fl t Ar trstr
>  .Ar command
> @@ -109,6 +109,8 @@ processes.
>  Enable (disable) tracing on the indicated process ID (only one
>  .Fl p
>  flag is permitted).
> +.It Fl T
> +Disable userland timekeeping, making time related system calls more 
> prevalent.
>  .It Fl t Ar trstr
>  Select which information to put into the dump file.
>  The argument can contain one or more of the following letters.
> Index: usr.bin/ktrace/ktrace.c
> ===
> RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 ktrace.c
> --- usr.bin/ktrace/ktrace.c   28 Jun 2019 13:35:01 -  1.36
> +++ usr.bin/ktrace/ktrace.c   13 Jul 2020 17:37:06 -
> @@ -100,7 +100,7 @@ main(int argc, char *argv[])
>   usage();
>   }
>   } else {
> - while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:")) != -1)
> + while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:T")) != -1)
>   switch ((char)ch) {
>   case 'a':
>   append = 1;
> @@ -140,6 +140,9 @@ main(int argc, char *argv[])
>   usage();
>   }
>   break;
> + case 'T':
> + putenv("LIBC_NOUSERTC=");
> + break;
>   default:
>   usage();
>   }
> @@ -240,9 +243,9 @@ usage(void)
>   " [-u trspec] command\n",
>   __progname);
>   else
> - fprintf(stderr, "usage: %s [-aBCcdi] [-f trfile] [-g pgid]"
> + fprintf(stderr, "usage: %s [-aCcdi] [-f trfile] [-g pgid]"
>   " [-p pid] [-t trstr]\n"
> - "   %s [-adi] [-f trfile] [-t trstr] command\n",
> + "   %s [-aBdiT] [-f trfile] [-t trstr] command\n",
>   __progname, __progname);
>   exit(1);
>  }



Re: disable libc sys wrappers?

2020-07-13 Thread Ted Unangst
On 2020-07-09, Theo de Raadt wrote:
> > Added a -T option to ktrace for transparency. I got ambitious here and made
> > it take suboptions, anticipating that other transparency modifications may
> > be desired.
> 
> Please don't do that.

Here is a simpler version.


Index: lib/libc/dlfcn/init.c
===
RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
retrieving revision 1.8
diff -u -p -r1.8 init.c
--- lib/libc/dlfcn/init.c   6 Jul 2020 13:33:05 -   1.8
+++ lib/libc/dlfcn/init.c   13 Jul 2020 17:36:04 -
@@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
_timekeep->tk_version != TK_VERSION)
_timekeep = NULL;
}
+   if (issetugid() == 0 && getenv("LIBC_NOUSERTC"))
+   _timekeep = NULL;
break;
}
}
Index: usr.bin/ktrace/ktrace.1
===
RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.1,v
retrieving revision 1.30
diff -u -p -r1.30 ktrace.1
--- usr.bin/ktrace/ktrace.1 15 May 2019 15:36:59 -  1.30
+++ usr.bin/ktrace/ktrace.1 13 Jul 2020 17:38:22 -
@@ -37,13 +37,13 @@
 .Nd enable kernel process tracing
 .Sh SYNOPSIS
 .Nm ktrace
-.Op Fl aBCcdi
+.Op Fl aCcdi
 .Op Fl f Ar trfile
 .Op Fl g Ar pgid
 .Op Fl p Ar pid
 .Op Fl t Ar trstr
 .Nm ktrace
-.Op Fl adi
+.Op Fl aBdiT
 .Op Fl f Ar trfile
 .Op Fl t Ar trstr
 .Ar command
@@ -109,6 +109,8 @@ processes.
 Enable (disable) tracing on the indicated process ID (only one
 .Fl p
 flag is permitted).
+.It Fl T
+Disable userland timekeeping, making time related system calls more prevalent.
 .It Fl t Ar trstr
 Select which information to put into the dump file.
 The argument can contain one or more of the following letters.
Index: usr.bin/ktrace/ktrace.c
===
RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.c,v
retrieving revision 1.36
diff -u -p -r1.36 ktrace.c
--- usr.bin/ktrace/ktrace.c 28 Jun 2019 13:35:01 -  1.36
+++ usr.bin/ktrace/ktrace.c 13 Jul 2020 17:37:06 -
@@ -100,7 +100,7 @@ main(int argc, char *argv[])
usage();
}
} else {
-   while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:")) != -1)
+   while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:T")) != -1)
switch ((char)ch) {
case 'a':
append = 1;
@@ -140,6 +140,9 @@ main(int argc, char *argv[])
usage();
}
break;
+   case 'T':
+   putenv("LIBC_NOUSERTC=");
+   break;
default:
usage();
}
@@ -240,9 +243,9 @@ usage(void)
" [-u trspec] command\n",
__progname);
else
-   fprintf(stderr, "usage: %s [-aBCcdi] [-f trfile] [-g pgid]"
+   fprintf(stderr, "usage: %s [-aCcdi] [-f trfile] [-g pgid]"
" [-p pid] [-t trstr]\n"
-   "   %s [-adi] [-f trfile] [-t trstr] command\n",
+   "   %s [-aBdiT] [-f trfile] [-t trstr] command\n",
__progname, __progname);
exit(1);
 }



Re: disable libc sys wrappers?

2020-07-10 Thread Theo de Raadt
Ingo Schwarze  wrote:

> Hi Theo,
> 
> Theo de Raadt wrote on Fri, Jul 10, 2020 at 10:02:46AM -0600:
> 
> > I also don't see the point of the leading _
> > 
> > Where does that come from?
> > 
> > This isn't a function namespace.  What does it signify, considering
> > no other environment variable uses _ prefixing.
> 
>$ man -kO Ev Ev~^_ | sed 's/ [^_][^ ]*//g'  # chicken scratches!
>   at, _
>   ksh, _
>   port-modules(5) _MODFOO_*
>   sudoers(5) _RLD*

Both of those are ports.  Neither are base.

> So, not absolutely accurate, but very close to the truth.  The
> bsd.port.mk(5) implementation, which does make extensive use of
> variable names with leading underscores in the sense intended by
> Ted (private variable, keep your fingers away!), is probably quite
> a special beast.

I don't agree at all, because this is not a programming namespace.



Re: disable libc sys wrappers?

2020-07-10 Thread Ingo Schwarze
Hi Theo,

Theo de Raadt wrote on Fri, Jul 10, 2020 at 10:02:46AM -0600:

> I also don't see the point of the leading _
> 
> Where does that come from?
> 
> This isn't a function namespace.  What does it signify, considering
> no other environment variable uses _ prefixing.

   $ man -kO Ev Ev~^_ | sed 's/ [^_][^ ]*//g'  # chicken scratches!
  at, _
  ksh, _
  port-modules(5) _MODFOO_*
  sudoers(5) _RLD*

So, not absolutely accurate, but very close to the truth.  The
bsd.port.mk(5) implementation, which does make extensive use of
variable names with leading underscores in the sense intended by
Ted (private variable, keep your fingers away!), is probably quite
a special beast.

> I think you are mixing too much stuff in here...

Yours,
  Ingo



Re: disable libc sys wrappers?

2020-07-10 Thread Theo de Raadt
I also don't see the point of the leading _

Where does that come from?

This isn't a function namespace.  What does it signify, considering
no other environment variable uses _ prefixing.

I think you are mixing too much stuff in here...



Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
> Added a -T option to ktrace for transparency. I got ambitious here and made
> it take suboptions, anticipating that other transparency modifications may
> be desired.

Please don't do that.

First off, I see no future in the open/openat idea.  Not just pledge,
not just unveil, not just reduction in ktrace clarity, I see no upside
and only downsides.

But secondly, I see no future in any other ideas of reducing system calls,
since they are the fundamental interface to do a request without magic.

This time thing is magic becuase it has to be (latency and accuracy),
but I see no future for anything else being magic.

If you make -T take options noone will use it, and the exercise is
pointless.  Noone is going to manually set the environment variable,
that much I can tell you also...




Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
> Call getenv before issetugid.

Why??

issetugid is a syscall (yes, it has overhead) which (amongst other things)
whether you can trust the environment.

But getenv -- that's a call which trusts enough of the environment array's
structure, to do a walk, and find.  Why do the walk, at all, if you aren't
supposed to?

It might not just be the string in the environment, it might be some other
corruption (I can't see one, but WHY do the idiom in a less safe way?)

It is entirely BACKWARDS from what we do elsewhere.

Who suggested you do this?




Re: disable libc sys wrappers?

2020-07-09 Thread Ted Unangst
On 2020-07-09, Theo de Raadt wrote:
> Hang on.  Do people ever want to force time calls, outside of ktrace?
> I doubt it.  Should ktrace maybe have a flag, similar to -B with LD_BIND_NOW,
> which sets the new environment variable?  Maybe -T?  The problem smells very
> similar to the root cause for LD_BIND_NOW setting..

So taking into account most feedback, another round.

env variable is now _LIBC_NOUSERTC. It's not libc's concern that ktrace may
want this, so I didn't go with that. The feature in question is usertc,
so that seems the most direct name.

Call getenv before issetugid.

Added a -T option to ktrace for transparency. I got ambitious here and made
it take suboptions, anticipating that other transparency modifications may
be desired. Fold in -B perhaps? I think typing -Tt is not so burdensome,
and saves us some alphabet down the line.

As a bonus, fix ktrace usage. -B only works with second synopsis, with command.


Index: lib/libc/dlfcn/init.c
===
RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
retrieving revision 1.8
diff -u -p -r1.8 init.c
--- lib/libc/dlfcn/init.c   6 Jul 2020 13:33:05 -   1.8
+++ lib/libc/dlfcn/init.c   10 Jul 2020 02:12:41 -
@@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
_timekeep->tk_version != TK_VERSION)
_timekeep = NULL;
}
+   if (getenv("_LIBC_NOUSERTC") && issetugid() == 0)
+   _timekeep = NULL;
break;
}
}
Index: usr.bin/ktrace/extern.h
===
RCS file: /home/cvs/src/usr.bin/ktrace/extern.h,v
retrieving revision 1.4
diff -u -p -r1.4 extern.h
--- usr.bin/ktrace/extern.h 26 Apr 2018 18:30:36 -  1.4
+++ usr.bin/ktrace/extern.h 10 Jul 2020 02:21:37 -
@@ -26,3 +26,4 @@
  */
 
 int getpoints(const char *, int);
+int gettrans(const char *, int);
Index: usr.bin/ktrace/ktrace.1
===
RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.1,v
retrieving revision 1.30
diff -u -p -r1.30 ktrace.1
--- usr.bin/ktrace/ktrace.1 15 May 2019 15:36:59 -  1.30
+++ usr.bin/ktrace/ktrace.1 10 Jul 2020 02:24:04 -
@@ -37,14 +37,15 @@
 .Nd enable kernel process tracing
 .Sh SYNOPSIS
 .Nm ktrace
-.Op Fl aBCcdi
+.Op Fl aCcdi
 .Op Fl f Ar trfile
 .Op Fl g Ar pgid
 .Op Fl p Ar pid
 .Op Fl t Ar trstr
 .Nm ktrace
-.Op Fl adi
+.Op Fl aBdi
 .Op Fl f Ar trfile
+.Op Fl T Ar transopts
 .Op Fl t Ar trstr
 .Ar command
 .Sh DESCRIPTION
@@ -109,6 +110,15 @@ processes.
 Enable (disable) tracing on the indicated process ID (only one
 .Fl p
 flag is permitted).
+.It Fl T Ar transopts
+Request additional transparency from libc.
+By default, no options are enabled.
+The argument can contain one or more of the following letters.
+.Pp
+.Bl -tag -width flag -offset indent -compact
+.It Cm t
+Disable userland timecounters so that time related system calls are visible.
+.El
 .It Fl t Ar trstr
 Select which information to put into the dump file.
 The argument can contain one or more of the following letters.
Index: usr.bin/ktrace/ktrace.c
===
RCS file: /home/cvs/src/usr.bin/ktrace/ktrace.c,v
retrieving revision 1.36
diff -u -p -r1.36 ktrace.c
--- usr.bin/ktrace/ktrace.c 28 Jun 2019 13:35:01 -  1.36
+++ usr.bin/ktrace/ktrace.c 10 Jul 2020 02:23:33 -
@@ -60,7 +60,7 @@ int
 main(int argc, char *argv[])
 {
enum { NOTSET, CLEAR, CLEARALL } clear;
-   int append, ch, fd, inherit, ops, pidset, trpoints;
+   int append, ch, fd, inherit, ops, pidset, trpoints, transparency;
pid_t pid;
char *tracefile, *tracespec;
mode_t omask;
@@ -71,6 +71,7 @@ main(int argc, char *argv[])
clear = NOTSET;
append = ops = pidset = inherit = pid = 0;
trpoints = is_ltrace ? KTRFAC_USER : DEF_POINTS;
+   transparency = 0;
tracefile = DEF_TRACEFILE;
tracespec = NULL;
 
@@ -100,7 +101,7 @@ main(int argc, char *argv[])
usage();
}
} else {
-   while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:")) != -1)
+   while ((ch = getopt(argc, argv, "aBCcdf:g:ip:t:T:")) != -1)
switch ((char)ch) {
case 'a':
append = 1;
@@ -140,6 +141,13 @@ main(int argc, char *argv[])
usage();
}
break;
+   case 'T':
+   transparency = gettrans(optarg, 0);
+   if (transparency < 0) {
+   warnx("unknown 

Re: disable libc sys wrappers?

2020-07-09 Thread Ingo Schwarze
Hi Mark,

Mark Kettenis wrote on Thu, Jul 09, 2020 at 07:59:25PM +0200:
> Ingo Schwrze wrote:

>> Now that i look at that, and at what you said previously, is it even
>> plausible that some user ever wants "-t c" ktracing but does
>> specifically *not* want to see clock_gettime(2) and friends?
[...]
> Yes.  If I ktrace something like chromium I really don't want to see
> the thousands of pointless clock_gettime(2) calls.

Fair enough.  Together with what deraadt@ explained,
it probably is -T, then.

>> Regarding the LD_ prefix, clock_gettime(2) and gettimeofday(2)
>> are in  and , respectively, but maybe a TIME_
>> prefix would be too specific.  Sure, it is the "time" sublibrary
>> of libc, but "TIME_" wouldn't really make it apparent that it
>> is related to libc at all.
>> 
>> So, should it instead be something like:
>> 
>>   LIBC_KTRACE_GETTIME   ?

> I wonder if it should start with an underscore instead to indicate
> that the variable is not for public consumption.  _KTRACE_TIME would
> do the job for me.  We can always rename it if we discover another use
> case apart from ktrace.

Sounds like user interface is almost finished; _KTRACE_TIME sounds
nicely concise, yet still properly specific.

Yours,
  Ingo



Re: disable libc sys wrappers?

2020-07-09 Thread Mark Kettenis
> Date: Thu, 9 Jul 2020 19:43:52 +0200
> From: Ingo Schwarze 
> 
> Hi Theo,
> 
> Theo de Raadt wrote on Thu, Jul 09, 2020 at 11:08:38AM -0600:
> > Ingo Schwarze  wrote:
> 
> >> So, what about
> >>   LD_KTRACE_GETTIME
> >> or a similar environment variable name?
> 
> > That naming seems logical.
> > 
> > If it is mostly hidden behind a ktrace flag (-T ?) then I think
> > we're in good shape.
> 
> Technically, the implementation of the new ktrace(1) flag will be
> totally different from the implementation of the existing ktrace -t
> flags.  But from the user perspective, the purpose of the new flag
> is just to "put some more information into the dump file", so
> shouldn't it be just another -t letter?  Like,
> 
>   c   trace system calls
>   T   trace clock_gettime(2) and gettimeofday(2)
> 
> or similar?
> 
> Now that i look at that, and at what you said previously, is it even
> plausible that some user ever wants "-t c" ktracing but does
> specifically *not* want to see clock_gettime(2) and friends?
> 
> If "i want system call ktracing" logically more or less implies "i
> also want to see clock_gettime and friends", then maybe we do not
> need a new command line flag at all, not even a sub-flag, and can
> instead just let "-t c" enable that, too?

Yes.  If I ktrace something like chromium I really don't want to see
the thousands of pointless clock_gettime(2) calls.

> 
> Regarding the LD_ prefix, clock_gettime(2) and gettimeofday(2)
> are in  and , respectively, but maybe a TIME_
> prefix would be too specific.  Sure, it is the "time" sublibrary
> of libc, but "TIME_" wouldn't really make it apparent that it
> is related to libc at all.
> 
> So, should it instead be something like:
> 
>   LIBC_KTRACE_GETTIME   ?

I wonder if it should start with an underscore instead to indicate
that the variable is not for public consumption.  _KTRACE_TIME would
do the job for me.  We can always rename it if we discover another use
case apart from ktrace.




Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
Ingo Schwarze  wrote:

> Hi Theo,
> 
> Theo de Raadt wrote on Thu, Jul 09, 2020 at 11:08:38AM -0600:
> > Ingo Schwarze  wrote:
> 
> >> So, what about
> >>   LD_KTRACE_GETTIME
> >> or a similar environment variable name?
> 
> > That naming seems logical.
> > 
> > If it is mostly hidden behind a ktrace flag (-T ?) then I think
> > we're in good shape.
> 
> Technically, the implementation of the new ktrace(1) flag will be
> totally different from the implementation of the existing ktrace -t
> flags.  But from the user perspective, the purpose of the new flag
> is just to "put some more information into the dump file", so
> shouldn't it be just another -t letter?  Like,
> 
>   c   trace system calls
>   T   trace clock_gettime(2) and gettimeofday(2)
> 
> or similar?

Every single one of the -t sub-optionsd are flags passed to the kernel,
and the kernel evaluates what to export.   This is completely not like -t.

> Now that i look at that, and at what you said previously, is it even
> plausible that some user ever wants "-t c" ktracing but does
> specifically *not* want to see clock_gettime(2) and friends?

Sorry, that isn't how this works.

> Unless i'm mistaken, we don't provide a way either to say "i want
> to see system calls, except that i don't want to see chdir(2),
> chmod(2), getuid(2), and mprotect(2)."

And that is not what is happening here.  This is a totally
new condition where libc has a way to "skip" doing "calls to system
calls" but we need a way to expose them *BECAUSE THE DEVELOPMENT PROCESS
NEEDS THE VISIBILITY*.



Re: disable libc sys wrappers?

2020-07-09 Thread Ingo Schwarze
Hi Theo,

Theo de Raadt wrote on Thu, Jul 09, 2020 at 11:08:38AM -0600:
> Ingo Schwarze  wrote:

>> So, what about
>>   LD_KTRACE_GETTIME
>> or a similar environment variable name?

> That naming seems logical.
> 
> If it is mostly hidden behind a ktrace flag (-T ?) then I think
> we're in good shape.

Technically, the implementation of the new ktrace(1) flag will be
totally different from the implementation of the existing ktrace -t
flags.  But from the user perspective, the purpose of the new flag
is just to "put some more information into the dump file", so
shouldn't it be just another -t letter?  Like,

c   trace system calls
T   trace clock_gettime(2) and gettimeofday(2)

or similar?

Now that i look at that, and at what you said previously, is it even
plausible that some user ever wants "-t c" ktracing but does
specifically *not* want to see clock_gettime(2) and friends?

If "i want system call ktracing" logically more or less implies "i
also want to see clock_gettime and friends", then maybe we do not
need a new command line flag at all, not even a sub-flag, and can
instead just let "-t c" enable that, too?

Keeping the user interface small is often good.  Needing only one
sub-flag rather than two to see all system calls doesn't seem that
bad, either.

Unless i'm mistaken, we don't provide a way either to say "i want
to see system calls, except that i don't want to see chdir(2),
chmod(2), getuid(2), and mprotect(2)."


Regarding the LD_ prefix, clock_gettime(2) and gettimeofday(2)
are in  and , respectively, but maybe a TIME_
prefix would be too specific.  Sure, it is the "time" sublibrary
of libc, but "TIME_" wouldn't really make it apparent that it
is related to libc at all.

So, should it instead be something like:

  LIBC_KTRACE_GETTIME   ?

Yours,
  Ingo



Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
Ingo Schwarze  wrote:

> Hi Theo,
> 
> Theo de Raadt wrote on Thu, Jul 09, 2020 at 09:47:26AM -0600:
> 
> > This time, they become invisible.
> [...]
> > There are many circumstances where ltrace is not suitable.
> [...]
> > We fiddle with programs all the time, to inspect them.
> 
> Fair enough, then.
> 
> The following variables already exist according to man -ak Ev~'\ 
>  * LD_PRELOAD
>  * LD_LIBRARY_PATH
>  * LD_BIND_NOW
>  * LD_TRACE_LOADED_OBJECTS
>  * LD_DEBUG
> 
> I susspect the name should:
> 
>  * start with "LD_"
>  * contain "TIME"
>  * contain "SYSCALL" or "KTRACE"

Actually the LD_ in those names refers to ld.so, but ld.so is not
making this decision.  libc is.

It also works for static binaries, because the checking position is
inside libc initialization.

That said, I don't like the originally proposed prefix STDIO_



Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
Ingo Schwarze  wrote:

> Hi Theo,
> 
> Theo de Raadt wrote on Thu, Jul 09, 2020 at 09:47:26AM -0600:
> 
> > This time, they become invisible.
> [...]
> > There are many circumstances where ltrace is not suitable.
> [...]
> > We fiddle with programs all the time, to inspect them.
> 
> Fair enough, then.
> 
> The following variables already exist according to man -ak Ev~'\ 
>  * LD_PRELOAD
>  * LD_LIBRARY_PATH
>  * LD_BIND_NOW
>  * LD_TRACE_LOADED_OBJECTS
>  * LD_DEBUG
> 
> I susspect the name should:
> 
>  * start with "LD_"
>  * contain "TIME"
>  * contain "SYSCALL" or "KTRACE"
> 
> If i understand correctly, what the user really wants is that
> time-related API calls appear in ktrace; that they cause system
> calls is merely how that is implemented.  Consequently, from the
> user perspective, "KTRACE" is more relevant than "SYSCALL".
> It is also clearer what it does; i assume using the word "SYSCALL"
> would require a lengthy wording like "FORCE_SYSCALL".
> 
> So, what about
> 
>   LD_KTRACE_GETTIME
> 
> or a similar environment variable name?

That naming seems logical.

If it is mostly hidden behind a ktrace flag (-T ?) then I think
we're in good shape.



Re: disable libc sys wrappers?

2020-07-09 Thread Ingo Schwarze
Hi Theo,

Theo de Raadt wrote on Thu, Jul 09, 2020 at 09:47:26AM -0600:

> This time, they become invisible.
[...]
> There are many circumstances where ltrace is not suitable.
[...]
> We fiddle with programs all the time, to inspect them.

Fair enough, then.

The following variables already exist according to man -ak Ev~'\

Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
Ingo Schwarze  wrote:

> Hi,
> 
> i wonder whether there is a layering violation going on here.
> 
> From the user perspective, whether an API call is a syscall or a
> mere library function doesn't make any difference, it's an
> implementation detail.

>From the developer's perspective it is a HUGE DEAL, because ktrace
now doesn't show calls to gettimeofday / clock_gettime calls in the
code most often traced (event loops).

> API calls have often moved from being library
> functions to syscall and vice versa, witnessed by the blurry line
> between sections 2 and 3 in the manual (which is not a problem at
> all).

When that happened, they remained visible.

This time, they become invisible.

o you say the user wants to see whether their program calls
> some time-related API functions, and in the past, ktrace was useful
> for that.  On a machine where syscalls for the purpose are no longer
> needed due to pirofti@'s optimization, does it really make sense to
> force useless syscalls just to be able to see the API calls in ktrace?
> Wouldn't it make more sense to instead simply use ltrace(1), which IIUC
> is intended for just that purpose, tracing API calls?

There are many circumstances where ltrace is not suitable.  It exposes
the wrong interface, and fails to show the parameters in the expected
fashion.

> Someone said setting up ltrace(1) is kind of harder, while ktrace(1)
> is very easy to use (i can confirm the latter, i use ktrace regularly).
> So maybe the real problem is to make sure ltrace(1) is about as easy
> to use as ktrace(1)?  (Not sure what exactly the problem(s) with ltrace
> usability are, if any, i didn't use it much so far.)

I don't want to see the layers of functions, which means I don't want
to use ltrace.  I want ktrace to be useful.

> It feels odd that a user would fiddle with the internal way how API
> calls are implemented, in particular using something as scary as
> environment variables, just to *inspect* what their program is doing...

We fiddle with programs all the time, to inspect them.  We add debug printf's!
How can doing this in a more subtle way be harmful?



Re: disable libc sys wrappers?

2020-07-09 Thread Ingo Schwarze
Hi,

i wonder whether there is a layering violation going on here.

>From the user perspective, whether an API call is a syscall or a
mere library function doesn't make any difference, it's an
implementation detail.  API calls have often moved from being library
functions to syscall and vice versa, witnessed by the blurry line
between sections 2 and 3 in the manual (which is not a problem at
all).

So you say the user wants to see whether their program calls
some time-related API functions, and in the past, ktrace was useful
for that.  On a machine where syscalls for the purpose are no longer
needed due to pirofti@'s optimization, does it really make sense to
force useless syscalls just to be able to see the API calls in ktrace?
Wouldn't it make more sense to instead simply use ltrace(1), which IIUC
is intended for just that purpose, tracing API calls?

Someone said setting up ltrace(1) is kind of harder, while ktrace(1)
is very easy to use (i can confirm the latter, i use ktrace regularly).
So maybe the real problem is to make sure ltrace(1) is about as easy
to use as ktrace(1)?  (Not sure what exactly the problem(s) with ltrace
usability are, if any, i didn't use it much so far.)

It feels odd that a user would fiddle with the internal way how API
calls are implemented, in particular using something as scary as
environment variables, just to *inspect* what their program is doing...
And that they would actually change what their program is doing just
for the purpose of that inspection...

Obviously, i don't object to any of this, and i may be totally wrong
trying to think about something so deep down in the system.  I'm just
wondering...

Yours,
  Ingo



Re: disable libc sys wrappers?

2020-07-09 Thread Mark Kettenis
> From: "Theo de Raadt" 
> Date: Thu, 09 Jul 2020 09:13:24 -0600
> 
> Ted Unangst  wrote:
> 
> > On 2020-07-08, Theo de Raadt wrote:
> > > I think we need something like this.
> > > 
> > > Documenting it will be a challenge.
> > > 
> > > I really don't like the name as is too generic, when the control is only
> > > for a narrow set of "current time" system calls.
> > 
> > I thought I'd start with something, but lots of questions.
> > 
> > Should it be per wrapper?
> 
> Perhaps not named per syscall, but named per grouping.  This causes
> direct syscalls for time retrieval.  Can you find a name for that?
> 
> > I know in the past we've had some similar
> > conversations about eliminating syscalls (open/openat)
> 
> I don't think we'll be doing that, because it doesn't really make the
> world any better and, as is evident with time handling mangles ktrace
> too much.
> 
> > Initial thought is it's easier to make one button, and then document
> > it in ktrace perhaps?
> 
> Probably in kdump also.
> 
> Hang on.  Do people ever want to force time calls, outside of ktrace?
> I doubt it.  Should ktrace maybe have a flag, similar to -B with LD_BIND_NOW,
> which sets the new environment variable?  Maybe -T?  The problem smells very
> similar to the root cause for LD_BIND_NOW setting..

Yes.  I think that makes lot of sense.  And I don't think we need to
document the envionment variable in that case.



Re: disable libc sys wrappers?

2020-07-09 Thread Theo de Raadt
Ted Unangst  wrote:

> On 2020-07-08, Theo de Raadt wrote:
> > I think we need something like this.
> > 
> > Documenting it will be a challenge.
> > 
> > I really don't like the name as is too generic, when the control is only
> > for a narrow set of "current time" system calls.
> 
> I thought I'd start with something, but lots of questions.
> 
> Should it be per wrapper?

Perhaps not named per syscall, but named per grouping.  This causes
direct syscalls for time retrieval.  Can you find a name for that?

> I know in the past we've had some similar
> conversations about eliminating syscalls (open/openat)

I don't think we'll be doing that, because it doesn't really make the
world any better and, as is evident with time handling mangles ktrace
too much.

> Initial thought is it's easier to make one button, and then document
> it in ktrace perhaps?

Probably in kdump also.

Hang on.  Do people ever want to force time calls, outside of ktrace?
I doubt it.  Should ktrace maybe have a flag, similar to -B with LD_BIND_NOW,
which sets the new environment variable?  Maybe -T?  The problem smells very
similar to the root cause for LD_BIND_NOW setting..



Re: disable libc sys wrappers?

2020-07-09 Thread Ted Unangst
On 2020-07-08, Theo de Raadt wrote:
> I think we need something like this.
> 
> Documenting it will be a challenge.
> 
> I really don't like the name as is too generic, when the control is only
> for a narrow set of "current time" system calls.

I thought I'd start with something, but lots of questions.

Should it be per wrapper? I know in the past we've had some similar
conversations about eliminating syscalls (open/openat) and I imagine
there will be future instances. Initial thought is it's easier to make
one button, and then document it in ktrace perhaps? But we can also add
per function options, and document them in the appropriate pages.



Re: disable libc sys wrappers?

2020-07-09 Thread Philip Guenther
On Wed, Jul 8, 2020 at 8:06 AM Theo de Raadt  wrote:

> Mark Kettenis  wrote:
>
> > > From: "Theo de Raadt" 
> > > Date: Wed, 08 Jul 2020 09:42:41 -0600
> > >
> > > I think we need something like this.
> > >
> > > Documenting it will be a challenge.
> > >
> > > I really don't like the name as is too generic, when the control is
> only
> > > for a narrow set of "current time" system calls.
> >
> > I'm not sure we should be using getenv() in this early initialization
> > function though.
>
> Ah, you worry about the static "#ifndef PIC / early_static_init" versus
> "PIC ld.so" environ setup, and this very early getenv() call might not be
> looking at the environ global.
>

It's late enough in the process (after a possible call
to early_static_init(), and definitely after any fixup by ld.so) that it
should work Just Fine.

I would flip the test to check the environment before running issetugid(2)
because the syscall is more expensive and it's nice not to clutter the
kdump output.  ;-)


Philip Guenther


Re: disable libc sys wrappers?

2020-07-08 Thread Todd C . Miller
On Wed, 08 Jul 2020 23:14:17 +0300, Paul Irofti wrote:

> I don't see the original mail here either. Is it me or Ted, or a forward from
>  a private conversation? Anyway, I am OK with this and Robert had a similar d
> iff two months ago when this started. Just make sure this is off by default f
> or both type of binaries.

Ted's original mail had an invalid Message-Id field which may have
caused delivery problems.

 - todd



Re: disable libc sys wrappers?

2020-07-08 Thread Theo de Raadt
Ted's new experimental mailer is incompatible with the real world.



Re: disable libc sys wrappers?

2020-07-08 Thread Paul Irofti
I don't see the original mail here either. Is it me or Ted, or a forward from a 
private conversation? Anyway, I am OK with this and Robert had a similar diff 
two months ago when this started. Just make sure this is off by default for 
both type of binaries.

Paul


În 8 iulie 2020 18:42:41 EEST, Theo de Raadt  a scris:
>I think we need something like this.
>
>Documenting it will be a challenge.
>
>I really don't like the name as is too generic, when the control is
>only
>for a narrow set of "current time" system calls.
>
>Ted Unangst  wrote:
>
>> Not sure how useful this will be, but I think it could be helpful to
>still
>> see section (2) functions in ktrace, even if there's magic to avoid
>that.
>> 
>> As proof of concept, if env LIBC_NOSYSWRAPPERS is set, the libc
>timecounters
>> are turned off. Now I see lots of gettimeofday syscalls in ktrace
>again.
>> 
>> Is this better than switching to ltrace? Combined ktrace and ltrace
>output
>> is fairly messy, but it seems to work. Setting it up to trace just a
>few
>> functions and all the system calls is a bit more involved.
>> 
>> 
>> Index: init.c
>> ===
>> RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
>> retrieving revision 1.8
>> diff -u -p -r1.8 init.c
>> --- init.c   6 Jul 2020 13:33:05 -   1.8
>> +++ init.c   8 Jul 2020 08:13:07 -
>> @@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
>>  _timekeep->tk_version != TK_VERSION)
>>  _timekeep = NULL;
>>  }
>> +if (issetugid() == 0 && getenv("LIBC_NOSYSWRAPPERS"))
>> +_timekeep = NULL;
>>  break;
>>  }
>>  }
>> 



Re: disable libc sys wrappers?

2020-07-08 Thread Theo de Raadt
Mark Kettenis  wrote:

> > From: "Theo de Raadt" 
> > Date: Wed, 08 Jul 2020 09:42:41 -0600
> > 
> > I think we need something like this.
> > 
> > Documenting it will be a challenge.
> > 
> > I really don't like the name as is too generic, when the control is only
> > for a narrow set of "current time" system calls.
> 
> I'm not sure we should be using getenv() in this early initialization
> function though.

Ah, you worry about the static "#ifndef PIC / early_static_init" versus
"PIC ld.so" environ setup, and this very early getenv() call might not be
looking at the environ global.

Ted, did you check static and dynamic binaries?



Re: disable libc sys wrappers?

2020-07-08 Thread Mark Kettenis
> From: "Theo de Raadt" 
> Date: Wed, 08 Jul 2020 09:42:41 -0600
> 
> I think we need something like this.
> 
> Documenting it will be a challenge.
> 
> I really don't like the name as is too generic, when the control is only
> for a narrow set of "current time" system calls.

I'm not sure we should be using getenv() in this early initialization
function though.

> Ted Unangst  wrote:
> 
> > Not sure how useful this will be, but I think it could be helpful to still
> > see section (2) functions in ktrace, even if there's magic to avoid that.
> > 
> > As proof of concept, if env LIBC_NOSYSWRAPPERS is set, the libc timecounters
> > are turned off. Now I see lots of gettimeofday syscalls in ktrace again.
> > 
> > Is this better than switching to ltrace? Combined ktrace and ltrace output
> > is fairly messy, but it seems to work. Setting it up to trace just a few
> > functions and all the system calls is a bit more involved.
> > 
> > 
> > Index: init.c
> > ===
> > RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
> > retrieving revision 1.8
> > diff -u -p -r1.8 init.c
> > --- init.c  6 Jul 2020 13:33:05 -   1.8
> > +++ init.c  8 Jul 2020 08:13:07 -
> > @@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
> > _timekeep->tk_version != TK_VERSION)
> > _timekeep = NULL;
> > }
> > +   if (issetugid() == 0 && getenv("LIBC_NOSYSWRAPPERS"))
> > +   _timekeep = NULL;
> > break;
> > }
> > }
> > 
> 
> 



Re: disable libc sys wrappers?

2020-07-08 Thread Theo de Raadt
I think we need something like this.

Documenting it will be a challenge.

I really don't like the name as is too generic, when the control is only
for a narrow set of "current time" system calls.

Ted Unangst  wrote:

> Not sure how useful this will be, but I think it could be helpful to still
> see section (2) functions in ktrace, even if there's magic to avoid that.
> 
> As proof of concept, if env LIBC_NOSYSWRAPPERS is set, the libc timecounters
> are turned off. Now I see lots of gettimeofday syscalls in ktrace again.
> 
> Is this better than switching to ltrace? Combined ktrace and ltrace output
> is fairly messy, but it seems to work. Setting it up to trace just a few
> functions and all the system calls is a bit more involved.
> 
> 
> Index: init.c
> ===
> RCS file: /home/cvs/src/lib/libc/dlfcn/init.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 init.c
> --- init.c6 Jul 2020 13:33:05 -   1.8
> +++ init.c8 Jul 2020 08:13:07 -
> @@ -114,6 +114,8 @@ _libc_preinit(int argc, char **argv, cha
>   _timekeep->tk_version != TK_VERSION)
>   _timekeep = NULL;
>   }
> + if (issetugid() == 0 && getenv("LIBC_NOSYSWRAPPERS"))
> + _timekeep = NULL;
>   break;
>   }
>   }
>