Re: Change behaviour of vis(3) in syslogd concerning backslash escaping
On 2022-06-02, Theo de Raadt wrote: But please consider this impact of the change you propose. > > There is one additional flag, VIS_NOSLASH, which inhibits the doubling of > backslashes and the backslash before the default format (that is, control > characters are represented by `^C' and meta characters as `M-C'). With > this flag set, the encoding is ambiguous and non-invertible. > > > This means if syslog is used to send some 'binary data', and you later on > want to decode meaning "unvis" the block, that won't work. Is that a usage > case to worry about? I think not. If you intentionally store binary data in syslog (why?) you can use base64. The vis call is to prevent harm, not guarantee it's unambiguous. Lots of potential syslog calls are already ambiguous if the user provides inputs containing spaces. A structured format like json seems like the right thing, so it'd be nice if syslog didn't interfere. At least when I looked at a similar problem recently, I cared about censoring the control characters, but not recovering them.
Re: tar extract to stdout
Err.
Index: extern.h
===
RCS file: /home/cvs/src/bin/pax/extern.h,v
retrieving revision 1.60
diff -u -p -r1.60 extern.h
--- extern.h23 Mar 2020 20:04:19 - 1.60
+++ extern.h27 May 2022 00:30:36 -
@@ -284,7 +284,7 @@ u_int st_hash(const char *, int, int);
/*
* tar.c
*/
-extern int tar_nodir;
+extern int tar_nodir, write_stdout;
extern char *gnu_name_string, *gnu_link_string;
int tar_endwr(void);
off_t tar_endrd(void);
Index: file_subs.c
===
RCS file: /home/cvs/src/bin/pax/file_subs.c,v
retrieving revision 1.55
diff -u -p -r1.55 file_subs.c
--- file_subs.c 23 Mar 2020 20:04:19 - 1.55
+++ file_subs.c 27 May 2022 00:26:39 -
@@ -68,6 +68,9 @@ file_creat(ARCHD *arcn)
mode_t file_mode;
int oerrno;
+ if (write_stdout)
+ return 1;
+
/*
* Assume file doesn't exist, so just try to create it, most times this
* works. We have to take special handling when the file does exist. To
@@ -940,7 +943,7 @@ file_write(int fd, char *str, int cnt, i
wcnt = MINIMUM(cnt, *rem);
cnt -= wcnt;
*rem -= wcnt;
- if (*isempt) {
+ if (*isempt && !write_stdout) {
/*
* have not written to this block yet, so we keep
* looking for zero's
Index: options.c
===
RCS file: /home/cvs/src/bin/pax/options.c,v
retrieving revision 1.103
diff -u -p -r1.103 options.c
--- options.c 15 Nov 2019 20:34:17 - 1.103
+++ options.c 28 May 2022 00:04:39 -
@@ -780,6 +780,8 @@ tar_options(int argc, char **argv)
pmtime = 0;
break;
case 'O':
+ /* extract to stdout, or write old style archives */
+ write_stdout = 1;
Oflag = 1;
break;
case 'o':
Index: tar.1
===
RCS file: /home/cvs/src/bin/pax/tar.1,v
retrieving revision 1.64
diff -u -p -r1.64 tar.1
--- tar.1 31 Mar 2022 17:27:14 - 1.64
+++ tar.1 28 May 2022 00:03:16 -
@@ -176,7 +176,8 @@ Do not preserve modification time.
Use only the numeric UID and GID values when creating or extracting an
archive.
.It Fl O
-Write old-style (non-POSIX) archives.
+When creating, write old-style (non-POSIX) archives.
+When extracting, write files to stdout.
.It Fl o
Don't write directory information that the older (V7) style
.Nm
Index: tar.c
===
RCS file: /home/cvs/src/bin/pax/tar.c,v
retrieving revision 1.70
diff -u -p -r1.70 tar.c
--- tar.c 1 Mar 2022 21:19:11 - 1.70
+++ tar.c 27 May 2022 00:34:06 -
@@ -73,6 +73,7 @@ static gid_t gid_warn;
*/
int tar_nodir; /* do not write dirs under old tar */
+int write_stdout; /* extract files to stdout */
char *gnu_name_string; /* GNU ././@LongLink hackery name */
char *gnu_link_string; /* GNU ././@LongLink hackery link */
Re: tar extract to stdout
On 2022-05-26, Todd C. Miller wrote: > Our tar's -O flag is only used when creating an archive, it is > unused for extraction. I'd prefer that we use the same option > letter as GNU tar and bsdtar for this. That would look something like this.
tar extract to stdout
tar can read and write archive files from stdin/out, but cannot extract files
to stdout. This may be kinda esoteric, but there's a few uses. Archive of log
files, etc., where you want to check for something without extracting to a
tmp file.
I tried working around this by renaming to /dev/fd/1, but no dice. tar can't
unlink that file, and so refuses to write to it.
And so, a -S option to write extracted files to stdout. gtar has a similar
option, but uses -O for the letter (already taken here).
Kind of a weird edge case, but the diff is not so very much code, and it's
not easy to workaround. I'd rather not install gtar just for such cases.
Index: extern.h
===
RCS file: /home/cvs/src/bin/pax/extern.h,v
retrieving revision 1.60
diff -u -p -r1.60 extern.h
--- extern.h23 Mar 2020 20:04:19 - 1.60
+++ extern.h27 May 2022 00:30:36 -
@@ -284,7 +284,7 @@ u_int st_hash(const char *, int, int);
/*
* tar.c
*/
-extern int tar_nodir;
+extern int tar_nodir, write_stdout;
extern char *gnu_name_string, *gnu_link_string;
int tar_endwr(void);
off_t tar_endrd(void);
Index: file_subs.c
===
RCS file: /home/cvs/src/bin/pax/file_subs.c,v
retrieving revision 1.55
diff -u -p -r1.55 file_subs.c
--- file_subs.c 23 Mar 2020 20:04:19 - 1.55
+++ file_subs.c 27 May 2022 00:26:39 -
@@ -68,6 +68,9 @@ file_creat(ARCHD *arcn)
mode_t file_mode;
int oerrno;
+ if (write_stdout)
+ return 1;
+
/*
* Assume file doesn't exist, so just try to create it, most times this
* works. We have to take special handling when the file does exist. To
@@ -940,7 +943,7 @@ file_write(int fd, char *str, int cnt, i
wcnt = MINIMUM(cnt, *rem);
cnt -= wcnt;
*rem -= wcnt;
- if (*isempt) {
+ if (*isempt && !write_stdout) {
/*
* have not written to this block yet, so we keep
* looking for zero's
Index: options.c
===
RCS file: /home/cvs/src/bin/pax/options.c,v
retrieving revision 1.103
diff -u -p -r1.103 options.c
--- options.c 15 Nov 2019 20:34:17 - 1.103
+++ options.c 27 May 2022 00:34:48 -
@@ -731,7 +731,7 @@ tar_options(int argc, char **argv)
* process option flags
*/
while ((c = getoldopt(argc, argv,
- "b:cef:hjmopqruts:vwxzBC:HI:LNOPXZ014578")) != -1) {
+ "b:cef:hjmopqruts:vwxzBC:HI:LNOPSXZ014578")) != -1) {
switch (c) {
case 'b':
/*
@@ -896,6 +896,9 @@ tar_options(int argc, char **argv)
*/
rmleadslash = 0;
break;
+ case 'S':
+ write_stdout = 1;
+ break;
case 'X':
/*
* do not pass over mount points in the file system
@@ -1694,10 +1697,10 @@ void
tar_usage(void)
{
(void)fputs(
- "usage: tar {crtux}[014578befHhjLmNOoPpqsvwXZz]\n"
+ "usage: tar {crtux}[014578befHhjLmNOoPpqSsvwXZz]\n"
" [blocking-factor | archive | replstr] [-C directory]
[-I file]\n"
" [file ...]\n"
- " tar {-crtux} [-014578eHhjLmNOoPpqvwXZz] [-b
blocking-factor]\n"
+ " tar {-crtux} [-014578eHhjLmNOoPpqSvwXZz] [-b
blocking-factor]\n"
" [-C directory] [-f archive] [-I file] [-s replstr]
[file ...]\n",
stderr);
exit(1);
Index: tar.1
===
RCS file: /home/cvs/src/bin/pax/tar.1,v
retrieving revision 1.64
diff -u -p -r1.64 tar.1
--- tar.1 31 Mar 2022 17:27:14 - 1.64
+++ tar.1 27 May 2022 00:33:25 -
@@ -32,7 +32,7 @@
.Sh SYNOPSIS
.Nm tar
.Sm off
-.No { Cm crtux No } Op Cm 014578befHhjLmNOoPpqsvwXZz
+.No { Cm crtux No } Op Cm 014578befHhjLmNOoPpqSsvwXZz
.Sm on
.Bk -words
.Op Ar blocking-factor | archive | replstr
@@ -43,7 +43,7 @@
.Nm tar
.No { Ns Fl crtux Ns }
.Bk -words
-.Op Fl 014578eHhjLmNOoPpqvwXZz
+.Op Fl 014578eHhjLmNOoPpqSvwXZz
.Op Fl b Ar blocking-factor
.Op Fl C Ar directory
.Op Fl f Ar archive
@@ -206,6 +206,8 @@ No more than one archive member is match
.Ar file .
When members of type directory are matched, the file hierarchy rooted at that
directory is also matched.
+.It Fl S
+Write output files to stdout instead of the file system when extracting.
.It Fl s Ar replstr
Modify the archive member names according to the substitution expression
.Ar replstr ,
Index: tar.c
===
RCS file: /home/cvs/src/bin/pax/tar.c,v
retrieving revision
Re: use cpu sensor for cpuspeed
On 2022-05-15, Mark Kettenis wrote: > > From: "Ted Unangst" > > Date: Sat, 14 May 2022 20:23:39 -0400 > > > > The cpu hz sensor is more accurate and updates faster than than the value > > currently used for hw.cpuspeed. So return that value (scaled). > > > > This doesn't set cpuspeed directly because the acpi does that and it's hard > > to create a whole system of priority overrides. I think it's simpler and > > maybe even better to track every value, and then return the best one > > available. > > At this point, I don't want this. Right now, it is actually useful > that hw.cpuspeed represents the requested P-state whereas the sensors > give us the "effective" frequency the core is actually running at. > And that is useful for evaluating what is actually happening. The man page should be updated then. HW_CPUSPEED (hw.cpuspeed) The current CPU frequency (in MHz).
use cpu sensor for cpuspeed
The cpu hz sensor is more accurate and updates faster than than the value
currently used for hw.cpuspeed. So return that value (scaled).
This doesn't set cpuspeed directly because the acpi does that and it's hard
to create a whole system of priority overrides. I think it's simpler and
maybe even better to track every value, and then return the best one
available.
Index: identcpu.c
===
RCS file: /home/cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.124
diff -u -p -r1.124 identcpu.c
--- identcpu.c 26 Apr 2022 10:48:20 - 1.124
+++ identcpu.c 15 May 2022 00:15:39 -
@@ -64,6 +64,7 @@ void cpu_check_vmm_cap(struct cpu_info *
/* sysctl wants this. */
char cpu_model[48];
int cpuspeed;
+uint64_t sensorspeed;
int amd64_has_xcrypt;
#ifdef CRYPTO
@@ -244,6 +245,8 @@ int
cpu_amd64speed(int *freq)
{
*freq = cpuspeed;
+ if (sensorspeed)
+ *freq = sensorspeed / 1ULL;
return (0);
}
@@ -337,6 +340,8 @@ cpu_hz_update_sensor(void *args)
val = (adelta * 100) / mdelta * tsc_frequency;
val = ((val + FREQ_50MHZ / 2) / FREQ_50MHZ) * FREQ_50MHZ;
ci->ci_hz_sensor.value = val;
+ if (CPU_IS_PRIMARY(ci))
+ sensorspeed = val;
}
atomic_clearbits_int(>p_flag, P_CPUPEG);
Re: another syzkaller problem in pf
On 2022-05-03, Moritz Buhl wrote: > Hi tech@, > > Syzkaller found a few crashes in pf_anchor_global_RB_REMOVE. > https://syzkaller.appspot.com/bug?id=a97f712331903ce38b8c084a489818b9bb5c6fcb > and also > https://syzkaller.appspot.com/text?tag=CrashLog=15ace9aaf0 > > The call stack is something like this: > pf_anchor_global_RB_REMOVE > pf_remove_if_empty_ruleset > pfi_dynaddr_setup > pfioctl > > I believe this is caused by two calls to pf_remove_if_empty_ruleset > in pfi_dynaddr_setup and a possibly pfr_destroy_ktable. > > I think it is a problem that pfr_attach_table is being called > with 1 (intr) because the introducing commit says: > > commit 4b3977248902c22d96aaebdb5784840debc2631c > Author: mikeb > Date: Mon Nov 24 13:22:09 2008 + > > Fix splasserts seen in pr 5987 by propagating a flag that discribes > whether we're called from the interrupt context to the functions > performing allocations. > > Looked at by mpf@ and henning@, tested by mpf@ and Antti Harri, > the pr originator. > > ok tedu > > And we are not in interrupt context. A detailed analysis follows. It's been a while, but iirc the flag is not necessarily whether this call, happening right now, is interrupt or not, but whether the paired alloc or free was or will be. So user code may need to set the intr flag if later an interrupt context will be touching this. (Interrupts are still not ever allowed to use user context allocators.) It may be more complicated than just flipping the flag, because there may be other ways to reach this free that require it. Or it could just be a bug. Just make sure we never free anything allocated with intr in this path.
no gawking in makesyscalls
I don't think we need to concern ourselves with cross awk compatibility here.
Despite the misleading comment, /usr/bin/awk supports toupper.
Index: makesyscalls.sh
===
RCS file: /home/cvs/src/sys/kern/makesyscalls.sh,v
retrieving revision 1.15
diff -u -p -r1.15 makesyscalls.sh
--- makesyscalls.sh 9 Dec 2021 00:26:10 - 1.15
+++ makesyscalls.sh 1 May 2022 04:45:49 -
@@ -70,28 +70,6 @@ sysent="sysent.switch"
trap "rm $sysdcl $sysprotos $sysent" 0
-# Awk program (must support nawk extensions)
-# Use "awk" at Berkeley, "nawk" or "gawk" elsewhere.
-awk=${AWK:-awk}
-
-# Does this awk have a "toupper" function? (i.e. is it GNU awk)
-isgawk=`$awk 'BEGIN { print toupper("true"); exit; }' 2>/dev/null`
-
-# If this awk does not define "toupper" then define our own.
-if [ "$isgawk" = TRUE ] ; then
- # GNU awk provides it.
- toupper=
-else
- # Provide our own toupper()
- toupper='
-function toupper(str) {
- _toupper_cmd = "echo "str" |tr a-z A-Z"
- _toupper_cmd | getline _toupper_str;
- close(_toupper_cmd);
- return _toupper_str;
-}'
-fi
-
# before handing it off to awk, make a few adjustments:
# (1) insert spaces around {, }, (, ), *, and commas.
# (2) get rid of any and all dollar signs (so that rcs id use safe)
@@ -111,8 +89,7 @@ s/\$//g
2,${
/^#/!s/\([{}()*,]\)/ \1 /g
}
-' < $2 | $awk "
-$toupper
+' < $2 | awk "
BEGIN {
# to allow nested #if/#else/#endif sets
savedepth = 0
Re: EVFILT_USER and kevent(2)
On 2022-04-30, Visa Hankala wrote: > I am in two minds about EVFILT_USER. On the one hand, having it on > OpenBSD might help with ports. On the other hand, it makes the kernel > perform a task that userspace can already handle using existing > interfaces. I agree you could do this with just a pipe, but also it's a bit simpler and it's not so very much code. It already exists, and seems at least one project uses it. I think it's fine. I also notice that libevent offers the event_active() function, which would allow building a similar mechanism, so there's more precedent for directly triggered events. (This function is not mentioned in the man page, however.)
Re: install btrace scripts
On 2022-04-30, Alexander Bluhm wrote: > Hi, > > Can we install the btrace scripts to /usr/share/btrace/ ? The > directory already exists, only the Makefile is not linked to the > build. > > And I would like to use #! to make them executable. It's weird to have exec files in share? I think it's not very hard to type "btrace script" vs script. Or have you added this directory to your PATH too? Either way, it would be helpful for the man page to mention this directory.
btrace hist printing large numbers
btrace needs a little more help printing very large numbers. If you have
something
like 2^61 in your histogram, it gets printed as 1K because that's what's left
over
after dividing by G and M. Add some more units, and it prints as 1E.
(It seems like there might be another bug, because I'm not really expecting
values
this large, but at least now I can see that they are large.)
Index: map.c
===
RCS file: /home/cvs/src/usr.sbin/btrace/map.c,v
retrieving revision 1.19
diff -u -p -r1.19 map.c
--- map.c 15 Nov 2021 14:57:57 - 1.19
+++ map.c 27 Apr 2022 08:04:10 -
@@ -267,11 +267,26 @@ hist_increment(struct hist *hist, const
long
hist_get_bin_suffix(long bin, char **suffix)
{
+#define EXA(PETA * 1024)
+#define PETA (TERA * 1024)
+#define TERA (GIGA * 1024)
#define GIGA (MEGA * 1024)
#define MEGA (KILO * 1024)
-#define KILO (1024)
+#define KILO (1024LL)
*suffix = "";
+ if (bin >= EXA) {
+ bin /= EXA;
+ *suffix = "E";
+ }
+ if (bin >= PETA) {
+ bin /= PETA;
+ *suffix = "P";
+ }
+ if (bin >= TERA) {
+ bin /= TERA;
+ *suffix = "T";
+ }
if (bin >= GIGA) {
bin /= GIGA;
*suffix = "G";
@@ -284,10 +299,7 @@ hist_get_bin_suffix(long bin, char **suf
bin /= KILO;
*suffix = "K";
}
-
return bin;
-#undef MEGA
-#undef KILO
}
/*
Re: pcb mutex userland
On 2022-03-17, Alexander Bluhm wrote:
> On Thu, Mar 17, 2022 at 01:07:12AM +0100, Mark Kettenis wrote:
> > > Date: Thu, 17 Mar 2022 01:01:46 +0100 (CET)
> > > From: Mark Kettenis
> > >
> > > > Date: Thu, 17 Mar 2022 00:47:15 +0100
> > > > From: Alexander Bluhm
> > > >
> > > > Hi,
> > > >
> > > > My previous atempt to add a mutex to in_pcb.h was reverted as it
> > > > broke userland build.
> > > >
> > > > Is the correct fix to include sys/mutex.h in every .c file that
> > > > includes netinet/in_pcb.h ? I made a release with it.
> > > > Or should I include sys/mutex.h in netinet/in_pcb.h ?
> > >
> > > Neither?
> > >
> > > It makes no sense to export the kernel mutex stuff to userland. Is
> > > there a way to avoid doing that by adding a bit for #ifdef _KERNEL?
> > ^
> > a bit more
>
> My diff adds struct mutex to struct inpcbtable. My later plan is
> to add a mutex also to struct inpcb.
>
> tcpbench uses libkvm to extract information from struct inpcbtable.
> netstat does that for struct inpcb. Also post mortem analysis from
> a kernel core dump is possible.
>
> I don't understand why userland must not know the size of struct
> mutex when tools where written to analyze these structs.
>
> Is there something special about struct mutex that should not shown
> to userland?
>
> Do you like this? Different structs for kernel and userland.
> I think this is confusing when used with libkvm.
>
> struct inpcbtable {
> TAILQ_HEAD(inpthead, inpcb) inpt_queue; /* [t] inet PCB queue */
> struct inpcbhead *inpt_hashtbl;/* [t] local and foreign hash
> */
> struct inpcbhead *inpt_lhashtbl; /* [t] local port hash */
> SIPHASH_KEY inpt_key, inpt_lkey;/* [t] secrets for hashes */
> u_long inpt_mask, inpt_lmask; /* [t] hash masks */
> int inpt_count, inpt_size; /* [t] queue count, hash size
> */
> #ifdef _KERNEL
> struct mutex inpt_mtx; /* protect queue and hash */
> #endif
> };
What if you add an #else case?
#ifdef _KERNEL
struct mutex inpt_mtx;
#else
char _mtxpad[8];
#endif
Re: document LOGIN_SETRTABLE
On 2022-03-03, Theo Buehler wrote: > This is missing in setusercontext(3). Of course. > > Index: gen/login_cap.3 > === > RCS file: /cvs/src/lib/libc/gen/login_cap.3,v > retrieving revision 1.18 > diff -u -p -r1.18 login_cap.3 > --- gen/login_cap.3 3 Jun 2021 13:38:18 - 1.18 > +++ gen/login_cap.3 3 Mar 2022 20:45:03 - > @@ -246,6 +246,9 @@ Sets the priority by > .It Dv LOGIN_SETRESOURCES > Sets the various system resources by > .Xr setrlimit 2 . > +.It Dv LOGIN_SETRTABLE > +Sets the routing table by > +.Xr setrtable 2 . > .It Dv LOGIN_SETUMASK > Sets the umask by > .Xr umask 2 . > @@ -261,6 +264,7 @@ Sets all of the above. > .Xr setlogin 2 , > .Xr setpriority 2 , > .Xr setrlimit 2 , > +.Xr setrtable 2 , > .Xr setuid 2 , > .Xr umask 2 , > .Xr initgroups 3 , > >
Re: vmd(8) fix error handling when hitting rlimit
On 2022-02-26, Dave Voutila wrote: > Following the discusion on misc@ and a diff from tedu@ [1], here's a bit > more work cleaning up the issue in vmd(8) to prevent vmd dying if a user > tries to create a vm with memory above the rlimit. > > I changed tedu's diff a bit to make it less verbose and print a value > that's human readable using fmt_scaled(3). It also only prints the > message about data limits if the error was ENOMEM. This does look better.
Re: Add rtable capability to login.conf
On 2022-02-06, Ted Unangst wrote:
> On 2022-02-05, Matthew Martin wrote:
> > On Sat, Jan 29, 2022 at 06:25:32PM -0600, Matthew Martin wrote:
> > > On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> > > > I believe it would be better to add setrtable to id pledge.
> >
> > ping
> >
> > Also are there any opinions on adding LOGIN_SETRTABLE to doas?
>
> I think this diff looks fine.
>
> For doas, we can use setall with an extra note in the man page.
Final auction for oks. I think all the login.conf.d changes are in now.
Plan is add setrtable to pledge first so people don't get caught, then libc.
>
>
> Index: doas.1
> ===
> RCS file: /home/cvs/src/usr.bin/doas/doas.1,v
> retrieving revision 1.25
> diff -u -p -r1.25 doas.1
> --- doas.116 Jan 2021 09:18:41 - 1.25
> +++ doas.16 Feb 2022 18:41:53 -
> @@ -54,6 +54,8 @@ and
> and the
> .Xr umask 2
> are set to values appropriate for the target user.
> +Other values may also be set as specified in
> +.Pa /etc/login.conf .
> .Ev DOAS_USER
> is set to the name of the user executing
> .Nm .
> Index: doas.c
> ===
> RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 doas.c
> --- doas.c30 Nov 2021 20:08:15 - 1.93
> +++ doas.c6 Feb 2022 18:39:38 -
> @@ -450,10 +450,7 @@ main(int argc, char **argv)
> if (targpw == NULL)
> errx(1, "no passwd entry for target");
>
> - if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
> - LOGIN_SETPATH |
> - LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
> - LOGIN_SETUSER) != 0)
> + if (setusercontext(NULL, targpw, target, LOGIN_SETALL) == -1)
> errx(1, "failed to set user context for target");
>
> if (pledge("stdio rpath exec", NULL) == -1)
>
>
Re: Add rtable capability to login.conf
On 2022-02-05, Matthew Martin wrote:
> On Sat, Jan 29, 2022 at 06:25:32PM -0600, Matthew Martin wrote:
> > On Sat, Jan 29, 2022 at 07:10:00PM -0500, Ted Unangst wrote:
> > > I believe it would be better to add setrtable to id pledge.
>
> ping
>
> Also are there any opinions on adding LOGIN_SETRTABLE to doas?
I think this diff looks fine.
For doas, we can use setall with an extra note in the man page.
Index: doas.1
===
RCS file: /home/cvs/src/usr.bin/doas/doas.1,v
retrieving revision 1.25
diff -u -p -r1.25 doas.1
--- doas.1 16 Jan 2021 09:18:41 - 1.25
+++ doas.1 6 Feb 2022 18:41:53 -
@@ -54,6 +54,8 @@ and
and the
.Xr umask 2
are set to values appropriate for the target user.
+Other values may also be set as specified in
+.Pa /etc/login.conf .
.Ev DOAS_USER
is set to the name of the user executing
.Nm .
Index: doas.c
===
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.93
diff -u -p -r1.93 doas.c
--- doas.c 30 Nov 2021 20:08:15 - 1.93
+++ doas.c 6 Feb 2022 18:39:38 -
@@ -450,10 +450,7 @@ main(int argc, char **argv)
if (targpw == NULL)
errx(1, "no passwd entry for target");
- if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
- LOGIN_SETPATH |
- LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
- LOGIN_SETUSER) != 0)
+ if (setusercontext(NULL, targpw, target, LOGIN_SETALL) == -1)
errx(1, "failed to set user context for target");
if (pledge("stdio rpath exec", NULL) == -1)
go, pledge, and dns
A go program that uses pledge("dns") mostly works except for two
incompatibilities with the way golang's dns library works. Otherwise
pledge("rpath") is required.
1. go likes to stat /etc/hosts to check for changes. I think this is
reasonable behavior. Patch below adds a whitelist to the kernel to permit
this. (libc does not currently cache results, but it could..?)
2. go tries to look a file called mdns.allow which does not exist on openbsd.
There are several platform dependent branches in go/src/net/conf.go, trying to
read this file should be avoided on openbsd. Patch left as an exercise.
Point 2 is also trivially worked around by performing a dummy lookup of
localhost before enabling pledge, so no urgency, but point 1 requires a code
change somewhere.
Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.278
diff -u -p -r1.278 kern_pledge.c
--- kern_pledge.c 20 Jan 2022 03:43:30 - 1.278
+++ kern_pledge.c 30 Jan 2022 21:01:43 -
@@ -733,12 +733,17 @@ pledge_namei(struct proc *p, struct name
break;
case SYS_stat:
- /* DNS needs /etc/resolv.conf. */
+ /* DNS needs /etc/{resolv.conf,hosts}. */
if ((ni->ni_pledge == PLEDGE_RPATH) &&
- (pledge & PLEDGE_DNS) &&
- strcmp(path, "/etc/resolv.conf") == 0) {
- ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
- return (0);
+ (pledge & PLEDGE_DNS)) {
+ if (strcmp(path, "/etc/resolv.conf") == 0) {
+ ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
+ return (0);
+ }
+ if (strcmp(path, "/etc/hosts") == 0) {
+ ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
+ return (0);
+ }
}
break;
}
Re: Add rtable capability to login.conf
I believe it would be better to add setrtable to id pledge.
On 2022-01-29, Matthew Martin wrote:
> It would be nice to have the ability to set a user's rtable upon login.
> This would be useful both for road warrior VPN setups (put both the VPN
> interface and user in an rdomain other than 0) and to differentiate
> users in firewall rules on the gateway or unbound views on a resolver.
> The below patch adds an rtable capability to login.conf which sets the
> user's default rtable on login.
>
> Since setusercontext will now call setrtable, that syscall needs to be
> covered under some pledge promise. Since the wroute promise already
> allows programs to set a different rtable on a per socket basis, I don't
> think adding the setrtable syscall materially changes the scope or
> attack surface. This impacts dhcpleased, iked, rad, and slaacd which
> already use the wroute promise. The wroute promise is added to
> calendar, skeyaudit, su, cron, and inetd.
>
> chroot, login, ssh, nsd, and unbound are impacted, but don't show up in
> the diff because they don't use pledge. ssh continues to work as
> expected: the user's session has the login specified default rtable;
> however, the socket connecting the user to the session remains in the
> original rtable so the connection doesn't break. nsd and unbound open
> their listening sockets prior to privdrop, so those sockets are in the
> original rtable (which should be set via rcctl). Notifications and XFRs
> for nsd and lookups for unbound happen in the login specified rtable.
> While this is somewhat surprising, I think most people would set the
> rtable for a daemon via rcctl.
>
> doas is a bit of a special case: it explicitly lists the LOGIN_ flags
> rather than using LOGIN_SETALL and removing specific flags. I am
> inclined to say doas should also set the default rtable, but it's not
> a strong stance. I've left it out of this version of the patch pending
> feedback on how others feel.
>
> In addition xenodm would need to add wroute to the pledge in dm.c
> StartDisplay and to the first pledge in session.c StartClient.
>
> From a Debian code search the ports which use LOGIN_SETALL appear to be
> mail/alpine, security/sudo, and x11/gnome/gdm. None of these ports have
> pledge markers in their Makefile.
>
> Finally it may be nicer to drop -a from calendar and skeyaudit. Each
> user that desires the service could then create their own cron job.
> While it would allow a fair bit of code to be deleted and a tighter
> pledge, it would break existing workflows and could be considered as
> in a separate thread if desired.
>
> - Matthew Martin
>
>
>
> diff --git include/login_cap.h include/login_cap.h
> index d9a4c2c349c..1e831b6471a 100644
> --- include/login_cap.h
> +++ include/login_cap.h
> @@ -53,7 +53,8 @@
> #define LOGIN_SETUMASK 0x0020 /* Set umask */
> #define LOGIN_SETUSER 0x0040 /* Set user */
> #define LOGIN_SETENV0x0080 /* Set environment */
> -#define LOGIN_SETALL0x00ff /* Set all. */
> +#define LOGIN_SETRTABLE 0x0100 /* Set rtable */
> +#define LOGIN_SETALL0x01ff /* Set all. */
>
> #define BI_AUTH "authorize" /* Accepted
> authentication */
> #define BI_REJECT "reject"/* Rejected
> authentication */
> diff --git lib/libc/gen/login_cap.c lib/libc/gen/login_cap.c
> index 862f33b2065..65848f09ef6 100644
> --- lib/libc/gen/login_cap.c
> +++ lib/libc/gen/login_cap.c
> @@ -52,6 +52,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -574,7 +575,7 @@ int
> setusercontext(login_cap_t *lc, struct passwd *pwd, uid_t uid, u_int flags)
> {
> login_cap_t *flc;
> - quad_t p;
> + quad_t p, rtable;
> int i;
>
> flc = NULL;
> @@ -625,6 +626,14 @@ setusercontext(login_cap_t *lc, struct passwd *pwd,
> uid_t uid, u_int flags)
> umask((mode_t)p);
> }
>
> + if (flags & LOGIN_SETRTABLE) {
> + rtable = login_getcapnum(lc, "rtable", 0, 0);
> +
> + if (setrtable((int)rtable) == -1) {
> + syslog(LOG_ERR, "%s: setrtable: %m", lc->lc_class);
> + }
> + }
> +
> if (flags & LOGIN_SETGROUP) {
> if (setresgid(pwd->pw_gid, pwd->pw_gid, pwd->pw_gid) == -1) {
> syslog(LOG_ERR, "setresgid(%u,%u,%u): %m",
> diff --git share/man/man5/login.conf.5 share/man/man5/login.conf.5
> index da935fa223e..ffeafc3531c 100644
> --- share/man/man5/login.conf.5
> +++ share/man/man5/login.conf.5
> @@ -276,6 +276,10 @@ Initial priority (nice) level.
> Require home directory to login.
> .\"
> .Pp
> +.It rtable Ta number Ta Dv 0 Ta
> +Rtable to be set for the class.
> +.\"
> +.Pp
> .It setenv Ta envlist Ta "" Ta
> A list of environment variables and associated values to be set for the
> class.
> .\"
> diff --git sys/kern/kern_pledge.c
Re: grep: add --null flag
On 2021-01-25, Sebastian Benoit wrote:
> Sebastian Benoit(be...@openbsd.org) on 2021.01.25 00:27:05 +0100:
> > Theo de Raadt(dera...@openbsd.org) on 2021.01.24 16:01:32 -0700:
> > > Stuart Henderson wrote:
> > >
> > > > On 2021/01/24 12:10, Theo de Raadt wrote:
> > > > > I completely despise that the option is called "--null".
> > > > >
> > > > > Someone was a complete idiot.
> > > >
> > > > gnu grep has both --null and -z for this (why do they do that?!).
> > > > If it's added as --null it should be added as -z too.
> > > >
> > > > Looking at Debian codesearch most things using it as --null use other
> > > > long options that we don't have. Maybe just adding as -z would be
> > > > enough. It does seem a useful and fairly widely supported feature.
> > >
> > > Yes, maybe just add -z.
> >
> > Actually it's "-Z, --null". The lowercase -z in gnu grep is
> >
> >-z, --null-data
> > Treat input and output data as sequences of lines, each
> > terminated by a zero byte (the ASCII NUL character) instead of
> > a newline. Like the -Z or --null option, this option can be
> > used with commands like sort-z to process arbitrary file
> > names.
>
> And we already have -z for "force grep to behave as zgrep".
>
> Diff below with tedu@ suggestion and changed manpage text.
What happened to -Z?
>
> >
> > Note that we have the -z in sort(1), which at least is consistent.
> > That also is a non-posix extension.
> >
> > > > Should have been -0 to match xargs and be similar to find's -print0
> > > > but it's too late for that now.
> > >
> > > Yes it should have been -0.
> > >
> > > But unfortunately some an uneducated idiot got involved. None of this
> > > is standardized. Unix script portability is being ruined by idiots, not
> > > just the people proposing it or writing it originally, but also the people
> > > who don't say "wrong" quickly enough. And much of this is because of
> > > intentional development silos.
>
> diff --git usr.bin/grep/grep.1 usr.bin/grep/grep.1
> index 5cc228df222..e1edae7e432 100644
> --- usr.bin/grep/grep.1
> +++ usr.bin/grep/grep.1
> @@ -49,6 +49,7 @@
> .Op Fl -context Ns Op = Ns Ar num
> .Op Fl -label Ns = Ns Ar name
> .Op Fl -line-buffered
> +.Op Fl -null
> .Op Ar pattern
> .Op Ar
> .Ek
> @@ -297,6 +298,16 @@ instead of the filename before lines.
> Force output to be line buffered.
> By default, output is line buffered when standard output is a terminal
> and block buffered otherwise.
> +.It Fl -null
> +Output a zero byte instead of the character that normally follows a
> +file name.
> +This option makes the output unambiguous, even in the presence of file
> +names containing unusual characters like newlines, making the output
> +suitable for use with the
> +.Fl 0
> +option to
> +.Xr xargs 1 .
> +This option is a non-POSIX extension and may not be portable.
> .El
> .Sh EXIT STATUS
> The
> diff --git usr.bin/grep/grep.c usr.bin/grep/grep.c
> index f41b5e20ca6..279d949fae7 100644
> --- usr.bin/grep/grep.c
> +++ usr.bin/grep/grep.c
> @@ -80,6 +80,7 @@ int vflag; /* -v: only show non-matching lines */
> int wflag; /* -w: pattern must start and end on word boundaries */
> int xflag; /* -x: pattern must match entire line */
> int lbflag;/* --line-buffered */
> +int nullflag; /* --null */
> const char *labelname; /* --label=name */
>
> int binbehave = BIN_FILE_BIN;
> @@ -89,6 +90,7 @@ enum {
> HELP_OPT,
> MMAP_OPT,
> LINEBUF_OPT,
> + NULL_OPT,
> LABEL_OPT,
> };
>
> @@ -134,6 +136,7 @@ static const struct option long_options[] =
> {"mmap",no_argument,NULL, MMAP_OPT},
> {"label", required_argument, NULL, LABEL_OPT},
> {"line-buffered", no_argument,NULL, LINEBUF_OPT},
> + {"null",no_argument,NULL, NULL_OPT},
> {"after-context", required_argument, NULL, 'A'},
> {"before-context", required_argument, NULL, 'B'},
> {"context", optional_argument, NULL, 'C'},
> @@ -436,6 +439,9 @@ main(int argc, char *argv[])
> case LINEBUF_OPT:
> lbflag = 1;
> break;
> + case NULL_OPT:
> + nullflag = 1;
> + break;
> case HELP_OPT:
> default:
> usage();
> diff --git usr.bin/grep/grep.h usr.bin/grep/grep.h
> index b3d24ae662b..37e295d4d40 100644
> --- usr.bin/grep/grep.h
> +++ usr.bin/grep/grep.h
> @@ -68,7 +68,7 @@ extern int cflags, eflags;
> extern intAflag, Bflag, Eflag, Fflag, Hflag, Lflag,
>Rflag, Zflag,
>bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
> - sflag, vflag, wflag, xflag;
> + sflag, vflag, wflag, xflag, nullflag;
> extern
Re: grep: add --null flag
On 2021-01-22, Omar Polo wrote:
>
> quasi three-weekly ping.
>
> Is this such a bad idea?
This seems reasonable. I think there's no need to change print line calls
though, just patch the implementation once.
>
> (TBH: I have still to look at how to write a regression test for this)
>
> Omar Polo writes:
>
> > Hello,
> >
> > There are various program, especially the one written with GNU grep in
> > mind, that expects various flags that grep in base doesn't have. While
> > some of the flags (like --color) can be easily worked out (i.e. by
> > patching/customising these programs) one thing that it isn't easily
> > workable is --null, because it alters the way grep outputs its data.
> >
> > --null makes grep output an ASCII NUL byte after the file name, so a
> > program can parse the output of grep unambiguously even when the file
> > names contains "funny" characters, such as a newline.
> >
> > GNU grep isn't the only one with a --null flag, also FreeBSD and NetBSD
> > grep do (at least by looking at their manpages), so it's somewhat
> > widespread.
> >
> > I searched the archives on marc.info but I haven't seen a previous
> > discussion about this.
> >
> > The following patch was tried against GNU grep (installed from packages)
> > and seem to behave consistently.
> >
> > I used the same text of the FreeBSD/NetBSD manpage for the description
> > of the --null option, but I really dislike it: it feels way to verbose
> > for what it's trying to say, but I wasn't able to come up with something
> > better.
> >
> > I'm not familiar at all with the grep codebase, so I hope I'm not
> > missing something. If this has some chances of being accepted, I guess
> > I should also add some regress test; I'll try to work on them soon, but
> > in the meantime I'm sending this.
> >
> > Thanks,
> >
> > Omar Polo
>
>
> diff e992327fc31d0277a6f8518613a7db1b9face78b /home/op/w/openbsd-src
> blob - 5cc228df222c54a0553f289b5da8bbbe6afd171e
> file + usr.bin/grep/grep.1
> --- usr.bin/grep/grep.1
> +++ usr.bin/grep/grep.1
> @@ -49,6 +49,7 @@
> .Op Fl -context Ns Op = Ns Ar num
> .Op Fl -label Ns = Ns Ar name
> .Op Fl -line-buffered
> +.Op Fl -null
> .Op Ar pattern
> .Op Ar
> .Ek
> @@ -297,6 +298,25 @@ instead of the filename before lines.
> Force output to be line buffered.
> By default, output is line buffered when standard output is a terminal
> and block buffered otherwise.
> +.It Fl -null
> +Output a zero byte (the ASCII NUL character) instead of the character
> +that normally follows a file name.
> +For example,
> +.Nm Fl l Fl -null
> +outputs a zero byte after each file name instead of the usual newline.
> +This option makes the output unambiguous, even in the presence of file
> +names containing unusual characters like newlines.
> +This option can be used with commands like
> +.Xr find 1
> +.Fl print0 Ns ,
> +.Xr perl 1
> +.Fl 0 Ns ,
> +.Xr sort 1
> +.Fl z Ns , and
> +.Xr args 1
> +.Fl 0
> +to process arbitrary file names, even those that contain newline
> +characters.
> .El
> .Sh EXIT STATUS
> The
> blob - f41b5e20ca68c9e9a36d2f7dd3c44329c621f29b
> file + usr.bin/grep/grep.c
> --- usr.bin/grep/grep.c
> +++ usr.bin/grep/grep.c
> @@ -80,6 +80,7 @@ int vflag; /* -v: only show non-matching lines */
> int wflag; /* -w: pattern must start and end on word boundaries */
> int xflag; /* -x: pattern must match entire line */
> int lbflag;/* --line-buffered */
> +int nullflag; /* --null */
> const char *labelname; /* --label=name */
>
> int binbehave = BIN_FILE_BIN;
> @@ -89,6 +90,7 @@ enum {
> HELP_OPT,
> MMAP_OPT,
> LINEBUF_OPT,
> + NULL_OPT,
> LABEL_OPT,
> };
>
> @@ -134,6 +136,7 @@ static const struct option long_options[] =
> {"mmap",no_argument,NULL, MMAP_OPT},
> {"label", required_argument, NULL, LABEL_OPT},
> {"line-buffered", no_argument,NULL, LINEBUF_OPT},
> + {"null",no_argument,NULL, NULL_OPT},
> {"after-context", required_argument, NULL, 'A'},
> {"before-context", required_argument, NULL, 'B'},
> {"context", optional_argument, NULL, 'C'},
> @@ -436,6 +439,9 @@ main(int argc, char *argv[])
> case LINEBUF_OPT:
> lbflag = 1;
> break;
> + case NULL_OPT:
> + nullflag = 1;
> + break;
> case HELP_OPT:
> default:
> usage();
> blob - b3d24ae662beb72c5632190c5c819bcc92f0389a
> file + usr.bin/grep/grep.h
> --- usr.bin/grep/grep.h
> +++ usr.bin/grep/grep.h
> @@ -68,7 +68,7 @@ extern int cflags, eflags;
> extern intAflag, Bflag, Eflag, Fflag, Hflag, Lflag,
>Rflag, Zflag,
>bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
> - sflag,
Re: doas sprinkle some more CFLAGS
On 2020-12-18, Martijn van Duren wrote: > So I ended up in doas again, this time with the CFLAGS I use for most of > my other projects. This popped up a few new not very exciting warnings. > Diff below compiles clean with both clang and gcc on amd64. > static int > match(uid_t uid, gid_t *groups, int ngroups, uid_t target, const char *cmd, > -const char **cmdargs, struct rule *r) > +const char * const*cmdargs, struct rule *r) That looks ugly, and I don't see the point. Const on the char prevents bugs, but after that we're just piling on crap.
Re: doas: improve error message
On 2020-10-09, Klemens Nanni wrote: > In case `cmd' and `args' in doas.conf(5) do not match, the generated > log message is unclear and might be read as if the command executed but > failed, i.e. returned non-zero: > > # cat /etc/doas.conf > permit nopass kn cmd echo args foo > $ doas echo foo > foo > $ doas echo bar > doas: Operation not permitted > > The corresponding syslog(3) messages from /var/log/secure: > > Oct 9 01:05:14 eru doas: kn ran command echo foo as root from /home/kn > Oct 9 01:05:20 eru doas: failed command for kn: echo bar > > The following reads unambiguous and better matches the EPERM wording: > > Oct 9 01:05:20 eru doas: command not permitted for kn: echo bar ok, i think that wording was just copy/paste.
Re: apmd(8) and hw.perfpolicy quirks
On 2020-09-27, Jeremie Courreges-Anglas wrote: > The diff below teaches apmd(8) -H to set hw.perfpolicy="manual" and > hw.setperf=100, instead of setting hw.perfpolicy="high". sure. if you would like to own this, by all means. :)
Re: apmd(8) and hw.perfpolicy quirks
On 2020-09-23, Jeremie Courreges-Anglas wrote: > ok? Seems fine. > Note: I inlined the apmd(8)->apm(8) perfpolicy conversion for now, which > brings a question. I find it weird that there is a special "high" > perfpolicy (effectively similar to perfpolicy=manual + setperf=100) but > no "low" perfpolicy. Is "high" useful to anyone? If you're benchmarking or something, it's convenient to toggle between high and auto. There's less use for low, since that's what auto defaults to.
Re: PATCH: better error return for exFAT filesystem
On 2020-08-09, Jonathan Gray wrote:
> mount_msdos(8) knows about EINVAL and will print "not an MSDOS filesystem"
I think mount_msdos could also inspect the filesytem for the common case of
exfat confusion.
Index: mount_msdos.c
===
RCS file: /home/cvs/src/sbin/mount_msdos/mount_msdos.c,v
retrieving revision 1.34
diff -u -p -r1.34 mount_msdos.c
--- mount_msdos.c 28 Jun 2019 13:32:45 - 1.34
+++ mount_msdos.c 9 Aug 2020 15:10:08 -
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -58,6 +59,7 @@ gid_t a_gid(char *);
uid_t a_uid(char *);
mode_t a_mask(char *);
void usage(void);
+intcheckexfat(const char *);
int
main(int argc, char **argv)
@@ -140,6 +142,9 @@ main(int argc, char **argv)
case EINVAL:
errcause =
"not an MSDOS filesystem";
+ if (checkexfat(dev)) {
+ errcause = "exFAT is not supported";
+ }
break;
default:
errcause = strerror(errno);
@@ -204,4 +209,20 @@ usage(void)
fprintf(stderr,
"usage: mount_msdos [-9ls] [-g gid] [-m mask] [-o options] [-u uid]
special node\n");
exit(1);
+}
+
+int
+checkexfat(const char *dev)
+{
+ char buf[4096];
+ int fd;
+
+ fd = open(dev, O_RDONLY);
+ if (fd != -1) {
+ ssize_t amt = read(fd, buf, sizeof(buf));
+ close(fd);
+ if (amt >= 11 && memcmp(buf + 3, "EXFAT ", 8) == 0)
+ return 1;
+ }
+ return 0;
}
Re: disable libc sys wrappers?
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?
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: pshared semaphores
On 2020-07-08, Mark Kettenis wrote:
> > From: "Ted Unangst"
> > Date: Wed, 08 Jul 2020 05:20:23 -0400
> >
> > On 2020-07-08, Ted Unangst wrote:
> > > I think this makes sem_init(pshared) work.
> >
> > Whoops, need more context to include the header file changes.
>
> It is a bit of a pity that we have to expose the internals here, but I
> don't see an easy way to avoid that, especially since hppa requires
> 16-byte alignment. At least doesn't do any
> namespace pollution as I believe a single underscore is enough here as
> everything is in file scope?
>
> This will require a libpthread major bump, and those are really
> painful! So I'm not sure we should do this just for pshared
> semaphores which hardly anybody uses.
>
> I wonder if we do this, should we include some additional padding in
> the struct for future expansion?
We can also expose only a padded struct sem { int _pad[4]; } or so.
That's a bit more cumbersome, and we need to be careful. But certainly
possible.
If this is useful, I think we should also consider removing the
indirection from pthread and mutex, etc. This is more work, but I
don't mind doing it all at once. Semaphore is just first priority.
(I think the mail issues are better now, thanks everyone.)
Re: disable libc sys wrappers?
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.
disable libc sys wrappers?
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;
}
}
pshared semaphores
I think this makes sem_init(pshared) work.
I have a test program from Lauri Tirkkonen and if I've understood it
correctly, it works now.
The essence of the diff is that we must eliminate the indirection so that
the application can properly allocate (mmap) the semaphore into shared memory.
There's two variations of the semaphore code (futex and compat). I tested the
compat on amd64, and think it works. The futex version also seems to work,
although I'm not sure how...
Index: rthread.h
===
RCS file: /home/cvs/src/lib/librthread/rthread.h,v
retrieving revision 1.64
diff -u -p -r1.64 rthread.h
--- rthread.h 13 Feb 2019 13:22:14 - 1.64
+++ rthread.h 8 Jul 2020 08:51:08 -
@@ -79,8 +79,8 @@ struct pthread_spinlock {
(((size) + (_thread_pagesize - 1)) & ~(_thread_pagesize - 1))
__BEGIN_HIDDEN_DECLS
-int_sem_wait(sem_t, int, const struct timespec *, int *);
-int_sem_post(sem_t);
+int_sem_wait(sem_t *, int, const struct timespec *, int *);
+int_sem_post(sem_t *);
void _rthread_init(void);
struct stack *_rthread_alloc_stack(pthread_t);
Index: rthread_sem.c
===
RCS file: /home/cvs/src/lib/librthread/rthread_sem.c,v
retrieving revision 1.32
diff -u -p -r1.32 rthread_sem.c
--- rthread_sem.c 6 Apr 2020 00:01:08 - 1.32
+++ rthread_sem.c 8 Jul 2020 08:58:07 -
@@ -55,7 +55,7 @@
* Internal implementation of semaphores
*/
int
-_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
+_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
int *delayed_cancel)
{
unsigned int val;
@@ -86,7 +86,7 @@ _sem_wait(sem_t sem, int can_eintr, cons
/* always increment count */
int
-_sem_post(sem_t sem)
+_sem_post(sem_t *sem)
{
membar_exit_before_atomic();
atomic_inc_int(>value);
@@ -98,70 +98,29 @@ _sem_post(sem_t sem)
* exported semaphores
*/
int
-sem_init(sem_t *semp, int pshared, unsigned int value)
+sem_init(sem_t *sem, int pshared, unsigned int value)
{
- sem_t sem;
if (value > SEM_VALUE_MAX) {
errno = EINVAL;
return (-1);
}
- if (pshared) {
- errno = EPERM;
- return (-1);
-#ifdef notyet
- char name[SEM_RANDOM_NAME_LEN];
- sem_t *sempshared;
- int i;
-
- for (;;) {
- for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
- name[i] = arc4random_uniform(255) + 1;
- name[SEM_RANDOM_NAME_LEN - 1] = '\0';
- sempshared = sem_open(name, O_CREAT | O_EXCL, 0, value);
- if (sempshared != SEM_FAILED)
- break;
- if (errno == EEXIST)
- continue;
- if (errno != EPERM)
- errno = ENOSPC;
- return (-1);
- }
-
- /* unnamed semaphore should not be opened twice */
- if (sem_unlink(name) == -1) {
- sem_close(sempshared);
- errno = ENOSPC;
- return (-1);
- }
-
- *semp = *sempshared;
- free(sempshared);
- return (0);
-#endif
- }
-
- sem = calloc(1, sizeof(*sem));
- if (!sem) {
- errno = ENOSPC;
- return (-1);
- }
+ memset(sem, 0, sizeof(*sem));
sem->value = value;
- *semp = sem;
+ sem->shared = pshared;
return (0);
}
int
-sem_destroy(sem_t *semp)
+sem_destroy(sem_t *sem)
{
- sem_t sem;
if (!_threads_ready) /* for SEM_MMAP_SIZE */
_rthread_init();
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -174,21 +133,19 @@ sem_destroy(sem_t *semp)
return (-1);
}
- *semp = NULL;
if (sem->shared)
munmap(sem, SEM_MMAP_SIZE);
else
- free(sem);
+ memset(sem, 0, sizeof(*sem));
return (0);
}
int
-sem_getvalue(sem_t *semp, int *sval)
+sem_getvalue(sem_t *sem, int *sval)
{
- sem_t sem;
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -199,11 +156,10 @@ sem_getvalue(sem_t *semp, int *sval)
}
int
-sem_post(sem_t *semp)
+sem_post(sem_t *sem)
{
- sem_t sem;
- if (!semp || !(sem = *semp)) {
+ if (!sem) {
errno = EINVAL;
return (-1);
}
@@ -214,11 +170,10 @@ sem_post(sem_t *semp)
}
int
-sem_wait(sem_t *semp)
+sem_wait(sem_t *sem)
{
struct tib *tib =
Re: pshared semaphores
On 2020-07-08, Ted Unangst wrote:
> I think this makes sem_init(pshared) work.
Whoops, need more context to include the header file changes.
Index: include/semaphore.h
===
RCS file: /home/cvs/src/include/semaphore.h,v
retrieving revision 1.1
diff -u -p -r1.1 semaphore.h
--- include/semaphore.h 15 Oct 2017 23:40:33 - 1.1
+++ include/semaphore.h 8 Jul 2020 08:47:06 -
@@ -38,10 +38,16 @@
#define _SEMAPHORE_H_
#include
+#include
/* Opaque type definition. */
-struct __sem;
-typedef struct __sem *sem_t;
+struct __sem {
+ _atomic_lock_t lock;
+ volatile int waitcount;
+ volatile int value;
+ int shared;
+};
+typedef struct __sem sem_t;
struct timespec;
#define SEM_FAILED ((sem_t *)0)
Index: lib/libc/include/thread_private.h
===
RCS file: /home/cvs/src/lib/libc/include/thread_private.h,v
retrieving revision 1.35
diff -u -p -r1.35 thread_private.h
--- lib/libc/include/thread_private.h 13 Feb 2019 13:22:14 - 1.35
+++ lib/libc/include/thread_private.h 8 Jul 2020 08:44:45 -
@@ -273,13 +273,6 @@ __END_HIDDEN_DECLS
#define_SPINLOCK_UNLOCKED _ATOMIC_LOCK_UNLOCKED
-struct __sem {
- _atomic_lock_t lock;
- volatile int waitcount;
- volatile int value;
- int shared;
-};
-
TAILQ_HEAD(pthread_queue, pthread);
#ifdef FUTEX
Index: lib/librthread/rthread.h
===
RCS file: /home/cvs/src/lib/librthread/rthread.h,v
retrieving revision 1.64
diff -u -p -r1.64 rthread.h
--- lib/librthread/rthread.h13 Feb 2019 13:22:14 - 1.64
+++ lib/librthread/rthread.h8 Jul 2020 09:18:34 -
@@ -79,8 +79,8 @@ struct pthread_spinlock {
(((size) + (_thread_pagesize - 1)) & ~(_thread_pagesize - 1))
__BEGIN_HIDDEN_DECLS
-int_sem_wait(sem_t, int, const struct timespec *, int *);
-int_sem_post(sem_t);
+int_sem_wait(sem_t *, int, const struct timespec *, int *);
+int_sem_post(sem_t *);
void _rthread_init(void);
struct stack *_rthread_alloc_stack(pthread_t);
Index: lib/librthread/rthread_sem.c
===
RCS file: /home/cvs/src/lib/librthread/rthread_sem.c,v
retrieving revision 1.32
diff -u -p -r1.32 rthread_sem.c
--- lib/librthread/rthread_sem.c6 Apr 2020 00:01:08 - 1.32
+++ lib/librthread/rthread_sem.c8 Jul 2020 09:18:34 -
@@ -55,7 +55,7 @@
* Internal implementation of semaphores
*/
int
-_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
+_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
int *delayed_cancel)
{
unsigned int val;
@@ -86,7 +86,7 @@ _sem_wait(sem_t sem, int can_eintr, cons
/* always increment count */
int
-_sem_post(sem_t sem)
+_sem_post(sem_t *sem)
{
membar_exit_before_atomic();
atomic_inc_int(>value);
@@ -98,70 +98,29 @@ _sem_post(sem_t sem)
* exported semaphores
*/
int
-sem_init(sem_t *semp, int pshared, unsigned int value)
+sem_init(sem_t *sem, int pshared, unsigned int value)
{
- sem_t sem;
if (value > SEM_VALUE_MAX) {
errno = EINVAL;
return (-1);
}
- if (pshared) {
- errno = EPERM;
- return (-1);
-#ifdef notyet
- char name[SEM_RANDOM_NAME_LEN];
- sem_t *sempshared;
- int i;
-
- for (;;) {
- for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
- name[i] = arc4random_uniform(255) + 1;
- name[SEM_RANDOM_NAME_LEN - 1] = '\0';
- sempshared = sem_open(name, O_CREAT | O_EXCL, 0, value);
- if (sempshared != SEM_FAILED)
- break;
- if (errno == EEXIST)
- continue;
- if (errno != EPERM)
- errno = ENOSPC;
- return (-1);
- }
-
- /* unnamed semaphore should not be opened twice */
- if (sem_unlink(name) == -1) {
- sem_close(sempshared);
- errno = ENOSPC;
- return (-1);
- }
-
- *semp = *sempshared;
- free(sempshared);
- return (0);
-#endif
- }
-
- sem = calloc(1, sizeof(*sem));
- if (!sem) {
- errno = ENOSPC;
- return (-1);
- }
+ memset(sem, 0, sizeof(*sem));
sem->value = value;
- *semp = sem;
+ sem->shared = pshared;
return (0);
}
int
-sem_destroy(sem_t *semp)
+sem_destroy(sem_t *sem)
{
- sem_t sem;
if (!_threads_ready)
Re: Avoid offline cpu in automatic frequency scheduling
On 2020-05-28, Solene Rapenne wrote: > > > In the current case, if you have offline cpu (because of hw.smt=0), > > > the total will still sum all the idle cpu and it's unlikely that > > > the total threshold is ever reached before the per cpu one. > > > > > > so, this diff skip offline cpus in the loop (robert@ gave me the big > > > clue to use cpu_is_online in the loop) This looks good. > > It's a lot of magic numbers without explanation here. I'm curious if > this could be improved. I guess that changing them would tip the > balance between responsiveness and battery saving. Did I do that? You have it about right, although there was never much science behind the algorithm or numbers. It was something that seemed to work okay without much complexity.
Re: Untangle proc * in VOP_*
On 2020-03-23, Martin Pieuchot wrote: > Most of the VOP_* methods include an argument of type "struct proc *" > called `a_p'. This pointer is always set to `curproc' as confirmed by > the diff below. The diff has been though base build with NFS on amd64 > and sparc64 as well as a full port bulk on amd64 and is in the current > octeon port bulk. > > I'd like to commit it in order to start removing the requirement of > passing a "struct proc *" pointer which is always set to curproc. This > is unnecessary and has spread in many other places in the kernel. > > That makes codes unnecessarily complicated: > - a "struct proc *" is carried over just to satisfy already complex >interfaces > - it isn't obvious which "struct proc *" is not the curproc and >asserts like KASSERT(p == curproc) are being made > - in many places where curproc is used people put XXX as if it was >wrong > > So this is just a starting diff, cleanups can start afterward. If this is a temporary measure, I think it should include some annotation to that effect, so that it doesn't continue spreading.
Re: librthread sem_t opaqueness, storage & unnamed semaphore sharing
On 2020-03-02, Lauri Tirkkonen wrote: > Thanks for the input, and ping - is there still something about this > diff that I should fix? I'm kinda busy, but I should be able to look into it eventually.
Re: librthread sem_t opaqueness, storage & unnamed semaphore sharing
Martin Pieuchot wrote: > On 24/02/20(Mon) 11:29, Lauri Tirkkonen wrote: > > On Mon, Feb 24 2020 10:24:53 +0100, Martin Pieuchot wrote: > > > On 23/02/20(Sun) 14:48, Lauri Tirkkonen wrote: > > > > I was working on a make jobserver implementation that uses POSIX > > > > semaphores as job tokens instead of a complicated socket-based approach. > > > > Initially I used named semaphores, which work fine, except if child > > > > processes with less privileges need to also open the named semaphore > > > > (eg. 'make build' as root executing 'su build -c make'). For that reason > > > > I wanted to use an unnamed semaphore (sem_init()) which is stored in shm > > > > -- that way I could leave the shm fd open and pass it to children. > > > > > > > > But unfortunately, sem_t is currently just a pointer to the opaque > > > > struct __sem, and sem_int() always calloc()s the storage for the struct. > > > > > > That's by design. > > > > Ok - could you elaborate what the design is? > > If the size of a descriptor change, because some fields are added and/or > removed, it doesn't matter for the application because it only manipulates > pointers. That means we can change the data types without creating an ABI > break. I think we are approaching the point where we can settle on fixed sized types now. If we want to be cautious, we can add a reserved padding field, too. But there are some edge cases which I think can be removed by eliminating the dynamic allocation paths. > > See the followup patch -- sharing the semaphore between processes does > > work with it. > > Well ignoring the `pshared' argument is questionable. Why don't you > remove the "#if notyet" and start playing with the existing code and > try to figure out if something is missing for your use case? I'm not sure the code in notyet will work. It was based on a misunderstanding I had of the requirements. Returning control of the sem_t placement to the application is the right approach.
refer to pointers as non-null
Noticed this in wait.2, though I imagine other occurences abound. I believe non-null is clearer when refering to a pointer than non-zero, which is a bit 80s, and less likely to be mistaken for the value pointed to. This same page also refers to non-zero values, fwiw. Index: wait.2 === RCS file: /home/cvs/src/lib/libc/sys/wait.2,v retrieving revision 1.30 diff -u -p -r1.30 wait.2 --- wait.2 1 May 2017 00:08:31 - 1.30 +++ wait.2 31 Jan 2020 10:28:42 - @@ -70,7 +70,7 @@ On return from a successful .Fn wait call, the .Fa status -area, if non-zero, is filled in with termination information about the +area, if non-null, is filled in with termination information about the process that exited (see below). .Pp The @@ -137,7 +137,7 @@ signal also have their status reported. .Pp If .Fa rusage -is non-zero, a summary of the resources used by the terminated +is non-null, a summary of the resources used by the terminated process and all its children is returned (this information is currently not available for stopped processes).
Re: Teach du(1) the -m flag, disk usage in megabytes
Jonathan Gray wrote: > On Sun, Jan 26, 2020 at 11:59:33AM -0500, David Goerger wrote: > > This diff teaches du(1) the -m flag, report disk usage in megabytes. > > This brings us in line with implementations in the other BSDs, Linux, > > and Illumos. > > Why is it needed? -k is required by POSIX, adding arguments for > megabytes, gigabytes, terabytes, petabytes etc seems silly when > there is already 512 byte blocks, kilobytes and -h output. there is also env BLOCKSIZE=1m although it's unwieldy to use environment. freebsd and linux both have -B blocksize which would be more flexible.
Re: apmd: fix autoaction timeout
Jeremie Courreges-Anglas wrote: > > The diff below improves the way apmd -z/-Z may trigger. > > I think the current behavior is bogus, incrementing and checking > apmtimeout like this doesn't make much sense. this all seems reasonable to me. > - I think we want some throttling mechanism, like wait for 1mn after we > resume before autosuspending again. But I want to fix obvious > problems first. would agree with that as well.
apmd poll every minute
Sometimes (particuarly if you are using apmd -z) the default polling interval of 10 minutes is a bit lazy and we fail to respond to low battery situations before they become no battery situations. Perhaps something smarter could be done, but the simplest change is to reduce the default polling interval to one minute. This is still rather slow, so as to not introduce unnecessary load on the system, but should make it more responsive to changing conditions. Index: apmd.8 === RCS file: /home/cvs/src/usr.sbin/apmd/apmd.8,v retrieving revision 1.50 diff -u -p -r1.50 apmd.8 --- apmd.8 4 Dec 2018 18:00:57 - 1.50 +++ apmd.8 25 Jan 2020 04:50:09 - @@ -111,7 +111,7 @@ periodically polls the APM driver for th If the battery charge level changes substantially or the external power status changes, the new status is logged. The polling rate defaults to -once per 10 minutes, but may be specified using the +once per 1 minute, but may be specified using the .Fl t command-line flag. .It Fl Z Ar percent Index: apmd.c === RCS file: /home/cvs/src/usr.sbin/apmd/apmd.c,v retrieving revision 1.91 diff -u -p -r1.91 apmd.c --- apmd.c 2 Nov 2019 00:41:36 - 1.91 +++ apmd.c 25 Jan 2020 04:49:54 - @@ -366,7 +366,7 @@ resumed(int ctl_fd) power_status(ctl_fd, 1, NULL); } -#define TIMO (10*60) /* 10 minutes */ +#define TIMO (1*60)/* 1 minute */ int main(int argc, char *argv[])
Re: acpivout(4): directly call ws_[gs]et_param
Patrick Wildt wrote: > acpivout_[gs]et_param don't know if they are being called by itself > or by someone else. I don't need to grab it again, I just need to > have it before calling an ACPI method. But when acpivout(4) calls > itself, it already has it, so taking it again would break it, as it's > non recursive. > > Since it's a global function pointer that hides the implementation, the > caller cannot just take the ACPI lock before calling ws_[gs]et_param. > The caller doesn't know that the implementation needs ACPI. On my X395 > the function set in ws_[gs]et_param have nothing to do with ACPI at all. Can you release the lock before calling ws_set_param?
Re: acme-client calloc fix
Matthew Martin wrote:
> On Wed, Jan 22, 2020 at 12:44:18AM -0500, Ted Unangst wrote:
> > should not size the size until the allocation succeeds, or the free path
> > will
> > try to deref the null array.
> >
> >
> > Index: json.c
> > ===
> > RCS file: /home/cvs/src/usr.sbin/acme-client/json.c,v
> > retrieving revision 1.14
> > diff -u -p -r1.14 json.c
> > --- json.c 18 Jun 2019 18:50:07 - 1.14
> > +++ json.c 22 Jan 2020 05:37:59 -
> > @@ -459,12 +459,13 @@ json_parse_order(struct jsmnn *n, struct
> > if ((array = json_getarray(n, "authorizations")) == NULL)
> > goto err;
> >
> > - if ((order->authsz = array->fields) > 0) {
> > + if (array->fields > 0) {
> > order->auths = calloc(sizeof(*order->auths), order->authsz);
>
> Shouldn't the second argument be switched to array->fields to maintain
> the same behavior?
thanks!
semicolon reduction
don't need semicolon after } in statements.
Index: ifconfig/brconfig.c
===
RCS file: /home/cvs/src/sbin/ifconfig/brconfig.c,v
retrieving revision 1.24
diff -u -p -r1.24 brconfig.c
--- ifconfig/brconfig.c 24 Oct 2019 18:54:10 - 1.24
+++ ifconfig/brconfig.c 22 Jan 2020 06:02:20 -
@@ -353,7 +353,7 @@ bridge_list(char *delim)
if ((1 << (v - 1)) & reqp->ifbr_protected)
printf(",%u", v);
}
- };
+ }
if (reqp->ifbr_ifsflags & IFBIF_STP)
printf(" %s role %s",
stpstates[reqp->ifbr_state],
Index: kbd/kbd_wscons.c
===
RCS file: /home/cvs/src/sbin/kbd/kbd_wscons.c,v
retrieving revision 1.33
diff -u -p -r1.33 kbd_wscons.c
--- kbd/kbd_wscons.c28 Jun 2019 13:32:44 - 1.33
+++ kbd/kbd_wscons.c22 Jan 2020 06:01:26 -
@@ -198,7 +198,7 @@ kbd_list(void)
default:
t = SA_MAX;
break;
- };
+ }
if (t != SA_MAX) {
kbds[t]++;
Index: mount_nfs/mount_nfs.c
===
RCS file: /home/cvs/src/sbin/mount_nfs/mount_nfs.c,v
retrieving revision 1.54
diff -u -p -r1.54 mount_nfs.c
--- mount_nfs/mount_nfs.c 5 Jan 2018 08:13:31 - 1.54
+++ mount_nfs/mount_nfs.c 22 Jan 2020 06:02:49 -
@@ -563,7 +563,7 @@ xdr_fh(XDR *xdrsp, struct nfhret *np)
if (!authfnd && (authcnt > 0 || np->auth != RPCAUTH_UNIX))
np->stat = EAUTH;
return (1);
- };
+ }
return (0);
}
Index: nfsd/nfsd.c
===
RCS file: /home/cvs/src/sbin/nfsd/nfsd.c,v
retrieving revision 1.39
diff -u -p -r1.39 nfsd.c
--- nfsd/nfsd.c 28 Jun 2019 13:32:45 - 1.39
+++ nfsd/nfsd.c 22 Jan 2020 05:59:42 -
@@ -135,7 +135,7 @@ main(int argc, char *argv[])
break;
default:
usage();
- };
+ }
argv += optind;
argc -= optind;
acme-client calloc fix
should not size the size until the allocation succeeds, or the free path will
try to deref the null array.
Index: json.c
===
RCS file: /home/cvs/src/usr.sbin/acme-client/json.c,v
retrieving revision 1.14
diff -u -p -r1.14 json.c
--- json.c 18 Jun 2019 18:50:07 - 1.14
+++ json.c 22 Jan 2020 05:37:59 -
@@ -459,12 +459,13 @@ json_parse_order(struct jsmnn *n, struct
if ((array = json_getarray(n, "authorizations")) == NULL)
goto err;
- if ((order->authsz = array->fields) > 0) {
+ if (array->fields > 0) {
order->auths = calloc(sizeof(*order->auths), order->authsz);
if (order->auths == NULL) {
warn("malloc");
goto err;
}
+ order->authsz = array->fields;
}
for (i = 0; i < array->fields; i++) {
Re: piixpm(4) on ATI SBx00
Karel Gardas wrote: > > > On 1/21/20 2:33 AM, Claudio Jeker wrote: > > Please test this since I can't test this properly at the moment. > > Would like to, but all hunks fail on today current: it's been committed.
Re: check hhmm for leave
Scott Cheloha wrote:
> > 1) it isn't documented that + can take a smaller number
> > 2) it will be hard to train people to use +0001
>
> These are my concerns as well.
>
> An idea I had a while back was to drop support for +hhmm and just
> support +minutes, like we do with shutdown(8). The invocation would
> then be:
this adds the check to just the clock time case. leave +mm isn't bad, but it's
a little awkward past 60. leave +180 for three hours? unfortunately that
clashes with existing usage. but my original concern is primarily with hhmm so
let's just address that.
Index: leave.c
===
RCS file: /home/cvs/src/usr.bin/leave/leave.c,v
retrieving revision 1.19
diff -u -p -r1.19 leave.c
--- leave.c 10 Feb 2018 00:00:47 - 1.19
+++ leave.c 22 Jan 2020 04:07:41 -
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
static __dead void usage(void);
@@ -83,6 +84,9 @@ main(int argc, char *argv[])
plusnow = 1;
++cp;
}
+
+ if (!plusnow && strlen(cp) != 4)
+ usage();
for (hours = 0; (c = *cp) && c != '\n'; ++cp) {
if (!isdigit((unsigned char)c))
check hhmm for leave
In testing leave, I found it accepts "12" and will alarm at 00:12, which is
probably not desirable. this check thats the specified time has 4 digits so
that the hour/minute math works properly.
Index: leave.c
===
RCS file: /home/cvs/src/usr.bin/leave/leave.c,v
retrieving revision 1.19
diff -u -p -r1.19 leave.c
--- leave.c 10 Feb 2018 00:00:47 - 1.19
+++ leave.c 22 Jan 2020 02:13:03 -
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
static __dead void usage(void);
@@ -83,6 +84,9 @@ main(int argc, char *argv[])
plusnow = 1;
++cp;
}
+
+ if (strlen(cp) != 4)
+ usage();
for (hours = 0; (c = *cp) && c != '\n'; ++cp) {
if (!isdigit((unsigned char)c))
leave.1
Rewrite some of the page to avoid second person. Index: leave.1 === RCS file: /home/cvs/src/usr.bin/leave/leave.1,v retrieving revision 1.16 diff -u -p -r1.16 leave.1 --- leave.1 13 Jul 2018 16:59:46 - 1.16 +++ leave.1 22 Jan 2020 02:08:00 - @@ -35,17 +35,17 @@ .Os .Sh NAME .Nm leave -.Nd remind you when you have to leave +.Nd print reminders when it is time to leave .Sh SYNOPSIS .Nm leave .Op Oo Cm + Oc Ns Ar hhmm .Sh DESCRIPTION .Nm leave -waits until the specified time, then reminds you that you -have to leave. -You are reminded 5 minutes and 1 minute before the actual +waits until the specified time, then prints a reminder +that it is time to leave. +Reminders are printed 5 minutes and 1 minute before the actual time, at the time, and every minute thereafter. -When you log off, +If the login session ends, .Nm leave exits just before it would have printed the next message. @@ -72,8 +72,7 @@ from the current time. .Pp If no argument is given, .Nm leave -prompts with "When do you -have to leave?". +prompts with "When do you have to leave?". A reply of newline causes .Nm leave to exit,
Re: [patch] signify's file name parsing broken
Ted Unangst wrote:
> MarcusMüller wrote:
> > I've just stumbled across a malfunction in signify: It cannot handle
> > file names that contain a `)` character, when checking a list of hashes
> > generated by `sha256` command line utilities (`sha256sum --tags` on
> > Linux).
>
> This fix is unfortunately rather complicated for the problem. Files with ) are
> not used within openbsd, for instance. It may possible to simplify it a bit?
Not much simpler, but maybe this is easier to follow. Keeps the hair in a
separate function.
Index: signify.c
===
RCS file: /home/cvs/src/usr.bin/signify/signify.c,v
retrieving revision 1.134
diff -u -p -r1.134 signify.c
--- signify.c 22 Dec 2019 06:37:25 - 1.134
+++ signify.c 21 Jan 2020 22:20:55 -
@@ -651,6 +651,37 @@ verifychecksum(struct checksum *c, int q
}
static void
+scanchecksum(const char *input, struct checksum *c)
+{
+ char *p, *algo, *file, *hash;
+ char line[2 * PATH_MAX];
+
+ if (strlcpy(line, input, sizeof(line)) >= sizeof(line))
+ goto fail;
+
+ /* algo (filename) = hash */
+ algo = line;
+ p = strchr(algo, ' ');
+ if (p == NULL || strncmp(p, " (", 2) != 0)
+ goto fail;
+ *p = 0;
+ file = p + 2;
+ p = strrchr(file, ')');
+ if (p == NULL || strncmp(p, ") = ", 4) != 0)
+ goto fail;
+ *p = 0;
+ hash = p + 4;
+
+ if (strlcpy(c->algo, algo, sizeof(c->algo)) >= sizeof(c->algo) ||
+ strlcpy(c->file, file, sizeof(c->file)) >= sizeof(c->file) ||
+ strlcpy(c->hash, hash, sizeof(c->hash)) >= sizeof(c->hash))
+ goto fail;
+ return;
+fail:
+ errx(1, "unable to parse checksum line %s", input);
+}
+
+static void
verifychecksums(char *msg, int argc, char **argv, int quiet)
{
struct ohash_info info = { 0, NULL, ecalloc, efree, NULL };
@@ -658,7 +689,7 @@ verifychecksums(char *msg, int argc, cha
struct checksum c;
char *e, *line, *endline;
int hasfailed = 0;
- int i, rv;
+ int i;
unsigned int slot;
ohash_init(, 6, );
@@ -675,13 +706,8 @@ verifychecksums(char *msg, int argc, cha
while (line && *line) {
if ((endline = strchr(line, '\n')))
*endline++ = '\0';
-#if PATH_MAX < 1024 || HASHBUFSIZE < 224
-#error sizes are wrong
-#endif
- rv = sscanf(line, "%31s (%1023[^)]) = %223s",
- c.algo, c.file, c.hash);
- if (rv != 3)
- errx(1, "unable to parse checksum line %s", line);
+
+ scanchecksum(line, );
line = endline;
if (argc) {
slot = ohash_qlookup(, c.file);
Re: key guessing for signify -C
Theo Buehler wrote:
> Two small things for signify -C:
>
> Contrary to what usage suggests, the -p pubkey argument for signify -C
> is optional in that signify will use the key specified in the untrusted
> comment. In -V mode, the key can be tied down a little by specifying -t.
>
> Right now, the -t keytype argument is silently ignored in -C mode:
>
> $ ftp
> https://cdn.openbsd.org/pub/OpenBSD/6.6/amd64/{INSTALL.amd64,SHA256.sig}
> >/dev/null
> $ signify -C -t pkg -x SHA256.sig INSTALL.amd64
> Signature Verified
> INSTALL.amd64: OK
>
> Diff below fixes usage and makes the above command error out with:
yes, that's an oversight. this looks good.
Re: [patch] signify's file name parsing broken
MarcusMüller wrote: > I've just stumbled across a malfunction in signify: It cannot handle > file names that contain a `)` character, when checking a list of hashes > generated by `sha256` command line utilities (`sha256sum --tags` on > Linux). This fix is unfortunately rather complicated for the problem. Files with ) are not used within openbsd, for instance. It may possible to simplify it a bit?
Re: nfs mp vnode list remove safe
Alexander Bluhm wrote:
> Hi,
>
> There is no remove and no sleep in this loop. So _SAFE is unnecessary.
>
> ok?
sure, with one bonus note.
>
> bluhm
>
> Index: nfs/nfs_subs.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_subs.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 nfs_subs.c
> --- nfs/nfs_subs.c10 Jan 2020 10:33:35 - 1.141
> +++ nfs/nfs_subs.c13 Jan 2020 18:02:54 -
> @@ -1509,16 +1509,16 @@ netaddr_match(int family, union nethosta
> void
> nfs_clearcommit(struct mount *mp)
> {
> - struct vnode *vp, *nvp;
> - struct buf *bp, *nbp;
> + struct vnode *vp;
> + struct buf *bp;
> int s;
>
> s = splbio();
> loop:
> - TAILQ_FOREACH_SAFE(vp, >mnt_vnodelist, v_mntvnodes, nvp) {
> + TAILQ_FOREACH(vp, >mnt_vnodelist, v_mntvnodes) {
> if (vp->v_mount != mp) /* Paranoia */
> goto loop;
that looks like an infinite loop? if there's a bad vnode on the list, it'll
still be there next time around.
Re: dangling vnode panic
Martin Pieuchot wrote: > On 09/01/20(Thu) 10:46, Alexander Bluhm wrote: > > Without this diff my regress machines became so unstable, that they > > often do not finish the test run. > > > > With this diff, they run fine. No regressions. > > Sadly this is a workaround. What is the issue? We're missing a barrier > during umount? How does other BSDs solved that? > > If your diff work because you changed a TAIL vs a HEAD insert and now > how to reproduce it you could capture stack traces from the function > doing the insert. Is it insmntque()? Could we prevent adding the vnode > to the list if a dying flag is on the mount point? I don't think that's possible. syncing the filesystem can cause more work to be queued. If you prevent adding the vnode to the list, how will that work be done? The alternative would be to add another loop around the list processing. I think that's worse because we don't know how many times to keep looping. bluhm's diff is the best approach imo. ok me.
Re: Scrolling in top(1)
Vadim Zhukov wrote: > Today I get really upset and angry due limitation of top(1): it shows > only hrrm, top processes (thank you, Chromium). Now here is a diff that > allows you to scroll process list by line or by half a screen. > > I've used the '0' and '9' keys to scroll down and up, respectively. > Unfortunately, 'k' is already taken, so vi-like binding to 'j'/'k' keys > is not possible. And emacs-style 'v'-for-all looks like too complex. > Anyone, who wants to use up/down and page up/page down keys, be my guest > for converting command_chars in top.c to using multi-byte sequences, > or whatever is needed for proper handling of those keys. > > Ideas, comments and (may I hope?) okays are welcome. :) One of each? It would be nice to extend this with some indication of where we are in the display (a first line that says skipping 19). I would have used ,.<> as navigation keys, but no matter. And it seems to work great, so ok.
Re: Add -R alias to -r for scp(1)
Kurt Mosiejczuk wrote: > cp(1) uses -R for recursive copy. scp(1) uses -r. This diff adds -R as an > alias for -r to scp(1) for those assuming consistency with cp(1). But it doesn't implement cp -R semantics. It does the copy the way cp -r does. (For symlinks, etc.)
Re: uvm_mapanon() & trylock
Martin Pieuchot wrote: > On 08/11/19(Fri) 17:54, Martin Pieuchot wrote: > > uvm_mapanon() is called in two occasions: mmap(2) and sigaltstack(2). > > Both code paths are obviously in process context an can sleep. That > > explains why none of them set the UVM_FLAG_TRYLOCK when calling such > > function. > > > > The diff below removes support for this flag. This introduces a > > difference with uvm_map(9) but simplifies the overall code. Removing > > support for an unneeded "try" variant also means the lock can be grabbed > > earlier. This is a requirement to not lose atomicity between > > uvm_mapanon() and uvm_map_pageable() in mmap(2). > > Anyone? yeah, this is partly why mapanon was split into another function, so we could remove the cruft that makes it complicated. go ahead.
Re: [PATCH] make: implement jobserver and use it to avoid exponential behavior
Marc Espie wrote: > reorganizing a large part of usr.bin or usr.sbin to just be one > single variation of bsd.prog.mk with multiple progs and multiple object > files... works just fine for, say 95% of the binaries in those directories > > (considering there are lots of directories with one single C file or two > C files, this sounds like a gain for make -jN, right ?) > > End result: no measurable gain. That on 4 cpu boxes at the time. > There are SMP issues to solve before this actually yields any > useful result... I don't know. I wrote a lua script that unrolled all the subdir and ran a single process make in each directory, up to 4 at a time, and got pretty good results. It takes 30s on my newer laptop just to run unbound configure. Nothing else is compiling during that time, and it can be. Just a few of those can shave minutes off a build. (This was before clang. The savings is somewhat diminished by the total time consumed by the gnu directory.)
Re: hexdump in boot loader
Alexander Bluhm wrote:
> +
> + for (n = 1; n < cmd.argc; n++) {
> + p = cmd.argv[n];
> + if (*p == '0') {
> + p++;
> + if (*p == 'x' || *p == 'X') {
> + p++;
> + b = 16;
> + } else
> + b = 8;
> + } else
> + b = 10;
> + val[n-1] = 0;
> + for (; *p != '\0'; p++) {
> + if (*p >= '0' && *p <= '9')
> + d = *p - '0';
> + else if (*p >= 'a' && *p <= 'z')
> + d = *p - 'a' + 10;
> + else if (*p >= 'A' && *p <= 'Z')
> + d = *p - 'A' + 10;
> + else
> + goto err;
> + if (d >= b)
> + goto err;
> + val[n-1] = val[n-1] * b + d;
why not use strtol here?
Re: Go vs uvm_map_inentry()
Martin Pieuchot wrote:
> The last, now reverted change, to uvm_map_inentry() exposes a race that
> is reproducible while building lang/go on amd64 which makes uvm_fault()
> fail, resulting a in a SIGSEV of at least one of the processes.
> Interestingly the machine cannot reproduce the race if the KERNEL_LOCK()
> is used as a barrier, like in the diff below.
>
> I couldn't reproduce the race with the go programs in src/regress.
> Building go is huge and difficult to instrument.
>
> By looking at sigpanic() in the traces it seems that stack manipulation
> is generally the trigger. I'm guessing the issue needs a multi-threaded
> program to be exposed but at that stage I'd appreciate any pointer or
> any simpler way to trigger the race.
Can you explain the race some more? The lock changes below would all seem to
come too late to resolve any races.
> if (uvm_map_inentry_recheck(serial, addr, ie)) {
> KERNEL_LOCK();
> + KERNEL_UNLOCK();
This in particular looks pretty weird.
Re: _pbuild user to have priority=5
Theo de Raadt wrote: > What about all the other users who aren't in staff? > > I think the approach is right. Push non-interactive down. The same then for src build user?
Re: uvm_map_inentry() & KERNEL_LOCK()
Theo de Raadt wrote: > In uvm_map_inentry_fix(), two variables in the map are now being read > without a per-map read lock, this was previously protected by the > kernel lock > > if (addr < map->min_offset || addr >= map->max_offset) > return (FALSE); > > When I wrote this I was told to either use KERNEL_LOCK() or > vm_map_lock_read(), which is vm_map_lock_read_ln() .. if > KERNEL_LOCK() is no longer good should vm_map_lock_read be moved > upwards, or are you confident those offsets are safe to read > independently without any locking?? indeed, another thread can expand the map, so that should have the read lock.
random safety for pbkdf
In the event that a program uses invalid parameters, I think we should overwrite the key with random data. Otherwise, there's a chance the program will continue with a zero key. It may even appear to work, encrypting and decrypting data, but with a weak key. Random data means it fails closed, and should also make it easier to detect such errors since it no longer interoperates. Index: bcrypt_pbkdf.c === RCS file: /home/cvs/src/lib/libutil/bcrypt_pbkdf.c,v retrieving revision 1.13 diff -u -p -r1.13 bcrypt_pbkdf.c --- bcrypt_pbkdf.c 12 Jan 2015 03:20:04 - 1.13 +++ bcrypt_pbkdf.c 15 Oct 2019 19:14:12 - @@ -110,10 +110,10 @@ bcrypt_pbkdf(const char *pass, size_t pa /* nothing crazy */ if (rounds < 1) - return -1; + goto bad; if (passlen == 0 || saltlen == 0 || keylen == 0 || keylen > sizeof(out) * sizeof(out)) - return -1; + goto bad; stride = (keylen + sizeof(out) - 1) / sizeof(out); amt = (keylen + stride - 1) / stride; @@ -166,4 +166,8 @@ bcrypt_pbkdf(const char *pass, size_t pa explicit_bzero(out, sizeof(out)); return 0; + +bad: + arc4random_buf(key, keylen); + return -1; } Index: pkcs5_pbkdf2.c === RCS file: /home/cvs/src/lib/libutil/pkcs5_pbkdf2.c,v retrieving revision 1.10 diff -u -p -r1.10 pkcs5_pbkdf2.c --- pkcs5_pbkdf2.c 18 Apr 2017 04:06:21 - 1.10 +++ pkcs5_pbkdf2.c 15 Oct 2019 19:17:08 - @@ -84,11 +84,11 @@ pkcs5_pbkdf2(const char *pass, size_t pa size_t r; if (rounds < 1 || key_len == 0) - return -1; + goto bad; if (salt_len == 0 || salt_len > SIZE_MAX - 4) - return -1; + goto bad; if ((asalt = malloc(salt_len + 4)) == NULL) - return -1; + goto bad; memcpy(asalt, salt, salt_len); @@ -118,4 +118,8 @@ pkcs5_pbkdf2(const char *pass, size_t pa explicit_bzero(obuf, sizeof(obuf)); return 0; + +bad: + arc4random_buf(key, key_len); + return -1; }
Re: top: align CPU lines horizontally
Klemens Nanni wrote: > On Sat, Oct 12, 2019 at 05:38:13PM -0500, Scott Cheloha wrote: > > Also, just count how many spaces we need to print ncpuonline, > > then use that when printing the individual CPU lines. > Yup, here's a minimal diff that does that without additional buffers and > globals but a single local static padding; it's definition looks lengthy > but that's what makes it nicely obvious (imho). > > Only downside compared to your diff is that we still assume machines to > have less than 1000 cores, that is keep combined stats at fixed "%-3d", > but that seems fairly acceptable to me. looks nicer and manages to remove more lines than it adds. :)
Re: top: align CPU lines horizontally
Klemens Nanni wrote: > On Sat, Oct 12, 2019 at 09:53:44AM -0600, Theo de Raadt wrote: > > I am suggesting you put the spaces after the cpu#. > Is this better? > > 4 CPUs: 0.0% user, 0.0% nice, 0.0% sys, 0.0% spin, 0.0% intr, 100% > idle > > CPU 0 : 0.0% user, 0.0% nice, 0.0% sys, 0.0% spin, 0.0% intr, 100% > idle > CPU 1 : 0.0% user, 0.0% nice, 0.0% sys, 0.0% spin, 0.0% intr, 100% > idle CPU 0:0.0% user, 0.0% nice, 0.0% sys, 0.0% spin, 0.0% intr, 100% idle CPU 10: 0.0% user, 0.0% nice, 0.0% sys, 0.0% spin, 0.0% intr, 100% idle CPU 100: 0.0% user, 0.0% nice, 0.0% sys, 0.0% spin, 0.0% intr, 100% idle
Re: more grep options (for zstdgrep)
While I'm looking at the man page, a minor clarification. "All long options are provided" can be read to mean that every gnu option is provided. This is not intended, and a simple reordering makes things clearer imo. Index: grep.1 === RCS file: /cvs/src/usr.bin/grep/grep.1,v retrieving revision 1.49 diff -u -p -r1.49 grep.1 --- grep.1 7 Oct 2019 20:51:34 - 1.49 +++ grep.1 7 Oct 2019 20:53:26 - @@ -364,7 +364,7 @@ are extensions to that specification, an .Fl f flag when used with an empty pattern file is left undefined. .Pp -All long options are provided for compatibility with +All provided long options are for compatibility with GNU versions of this utility. .Pp Historic versions of the
more grep options (for zstdgrep)
The zstd package includes a zstdgrep script, which should behave like zgrep,
however it assumes a few gnu grep behaviors we don't support.
1. --label=name prints a custom label, so you can get file.txt, not
file.txt.zst in the output.
2. It uses - for stdin instead of a missing argument.
Both are easy to address, as below. With this, I can grep my old log files
with having to do the zstdcat | grep dance myself.
Index: grep.1
===
RCS file: /home/cvs/src/usr.bin/grep/grep.1,v
retrieving revision 1.47
diff -u -p -r1.47 grep.1
--- grep.1 18 Jul 2019 15:32:50 - 1.47
+++ grep.1 6 Oct 2019 18:21:05 -
@@ -47,6 +47,7 @@
.Op Fl m Ar num
.Op Fl -binary-files Ns = Ns Ar value
.Op Fl -context Ns Op = Ns Ar num
+.Op Fl -label Ar name
.Op Fl -line-buffered
.Op Ar pattern
.Op Ar
@@ -283,6 +284,10 @@ do not search binary files;
and
.Ar text :
treat all files as text.
+.It Fl -label Ar name
+Print
+.Ar name
+instead of the filename before lines.
.It Fl -line-buffered
Force output to be line buffered.
By default, output is line buffered when standard output is a terminal
Index: grep.c
===
RCS file: /home/cvs/src/usr.bin/grep/grep.c,v
retrieving revision 1.60
diff -u -p -r1.60 grep.c
--- grep.c 18 Jul 2019 15:32:50 - 1.60
+++ grep.c 6 Oct 2019 18:13:02 -
@@ -80,6 +80,7 @@ intvflag; /* -v: only show non-matchi
int wflag; /* -w: pattern must start and end on word boundaries */
int xflag; /* -x: pattern must match entire line */
int lbflag;/* --line-buffered */
+const char *labelname; /* --label=name */
int binbehave = BIN_FILE_BIN;
@@ -87,7 +88,8 @@ enum {
BIN_OPT = CHAR_MAX + 1,
HELP_OPT,
MMAP_OPT,
- LINEBUF_OPT
+ LINEBUF_OPT,
+ LABEL_OPT,
};
/* Housekeeping */
@@ -130,6 +132,7 @@ static const struct option long_options[
{"binary-files",required_argument, NULL, BIN_OPT},
{"help",no_argument,NULL, HELP_OPT},
{"mmap",no_argument,NULL, MMAP_OPT},
+ {"label", required_argument, NULL, LABEL_OPT},
{"line-buffered", no_argument,NULL, LINEBUF_OPT},
{"after-context", required_argument, NULL, 'A'},
{"before-context", required_argument, NULL, 'B'},
@@ -427,6 +430,9 @@ main(int argc, char *argv[])
case MMAP_OPT:
/* default, compatibility */
break;
+ case LABEL_OPT:
+ labelname = optarg;
+ break;
case LINEBUF_OPT:
lbflag = 1;
break;
@@ -458,6 +464,11 @@ main(int argc, char *argv[])
if (argc != 0 && needpattern) {
add_patterns(*argv);
+ --argc;
+ ++argv;
+ }
+ if (argc == 1 && strcmp(*argv, "-") == 0) {
+ /* stdin */
--argc;
++argv;
}
Index: grep.h
===
RCS file: /home/cvs/src/usr.bin/grep/grep.h,v
retrieving revision 1.26
diff -u -p -r1.26 grep.h
--- grep.h 23 Jan 2019 23:00:54 - 1.26
+++ grep.h 6 Oct 2019 18:09:32 -
@@ -70,6 +70,7 @@ extern int Aflag, Bflag, Eflag, Fflag,
bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
sflag, vflag, wflag, xflag;
extern int binbehave;
+extern const char *labelname;
extern int first, matchall, patterns, tail, file_err;
extern char**pattern;
Index: util.c
===
RCS file: /home/cvs/src/usr.bin/grep/util.c,v
retrieving revision 1.60
diff -u -p -r1.60 util.c
--- util.c 17 Jul 2019 04:24:20 - 1.60
+++ util.c 6 Oct 2019 18:10:07 -
@@ -122,6 +122,8 @@ procfile(char *fn)
}
ln.file = fn;
+ if (labelname)
+ ln.file = (char *)labelname;
ln.line_no = 0;
ln.len = 0;
linesqueued = 0;
Re: pretty borders for slitherins
Nicholas Marriott wrote: > OK seems like it is always using ACS which is correct. > > I just remembered jmc asked me about this before and it turned out that > ACS doesn't work with the DRM console - I don't think the font have the > right symbol for either ACS or UTF-8 line drawing, but there may be > other problems in there as well. > > If you are using DRM you can set TERM to pccon0 which is a variant with > ASCII line drawing and ncurses will use ASCII for line drawing. ah, yes, env TERM=pccon0 causes correct fallback to --|| style lines.
Re: pretty borders for slitherins
Scott Cheloha wrote: > On Mon, Sep 23, 2019 at 06:23:32PM -0400, Ted Unangst wrote: > > snake and worm draw boxes, but they can be prettier by using the default > > style, which will use line drawing instead of ugly -*| characters. > > > > should do the right thing on a vt100, but only tested in xterm. > > It looks much prettier in an xterm but in the system console I'm > getting question marks. Is that a limitation of the terminal or > am I missing the glyphs? Dunno if this is the expected behavior > but it is uglier than what we had before. ah, that's disappointing. my understanding is it should revert to ascii, but there seems to be a flaw, either in curses or my understanding. curses!
pretty borders for slitherins
snake and worm draw boxes, but they can be prettier by using the default
style, which will use line drawing instead of ugly -*| characters.
should do the right thing on a vt100, but only tested in xterm.
Index: snake/snake.c
===
RCS file: /home/cvs/src/games/snake/snake.c,v
retrieving revision 1.34
diff -u -p -r1.34 snake.c
--- snake/snake.c 28 Jun 2019 13:32:52 - 1.34
+++ snake/snake.c 13 Sep 2019 17:05:19 -
@@ -450,23 +450,8 @@ setup(void)
pchar([i], SNAKETAIL);
}
pchar([0], SNAKEHEAD);
- drawbox();
+ border(0, 0, 0, 0, 0, 0, 0, 0);
refresh();
-}
-
-void
-drawbox(void)
-{
- int i;
-
- for (i = 1; i <= ccnt; i++) {
- mvaddch(0, i, '-');
- mvaddch(lcnt + 1, i, '-');
- }
- for (i = 0; i <= lcnt + 1; i++) {
- mvaddch(i, 0, '|');
- mvaddch(i, ccnt + 1, '|');
- }
}
void
Index: worm/worm.c
===
RCS file: /home/cvs/src/games/worm/worm.c,v
retrieving revision 1.39
diff -u -p -r1.39 worm.c
--- worm/worm.c 24 Aug 2018 11:14:49 - 1.39
+++ worm/worm.c 13 Sep 2019 16:51:39 -
@@ -122,7 +122,7 @@ main(int argc, char **argv)
}
stw = newwin(1, COLS-1, 0, 0);
tv = newwin(LINES-1, COLS-1, 1, 0);
- box(tv, '*', '*');
+ box(tv, 0, 0);
scrollok(tv, FALSE);
scrollok(stw, FALSE);
wmove(stw, 0, 0);
Re: msdosfs: remove timezone support
Scott Cheloha wrote: > The FAT file system hails from Redmond and so it tracks a timezone. > OpenBSD implicitly uses the kernel timezone when selecting which > timezone to use when mounting a FAT file system. > > This support is undocumented. > > This patch removes that support. > > The upshot is that your timestamps will be off if (a) you were making > use of the kernel timezone in OpenBSD and (b) the file system in > question is shared between machines that vary in their support for FAT > timezones. oh, and it seems bugged if your uptime crosses a DST transition? good riddance i guess.
Re: Removing the kernel timezone: date(1): drop -d dst and -t minutes_west
Scott Cheloha wrote:
> - while ((ch = getopt(argc, argv, "ad:f:jr:ut:z:")) != -1)
> + while ((ch = getopt(argc, argv, "af:jr:uz:")) != -1)
> switch(ch) {
> case 'a':
> slidetime = 1;
> break;
> - case 'd': /* daylight saving time */
> - tz.tz_dsttime = atoi(optarg) ? 1 : 0;
> - break;
> case 'f': /* parse with strptime */
> pformat = optarg;
> break;
as for the diff, this one looks good.
Re: Removing the kernel timezone: date(1): drop -d dst and -t minutes_west
Scott Cheloha wrote: > It doesn't mean anything. I guess I'm still gunshy about removing > options and breaking things after the lock(1) thing. If the default behavior changes, and the option is now meaningless, but still results in the *same* behavior, keep the option. The user still obtains the desired result. If the code that makes the option work has been deleted, and the behavior will change, delete the option. The user is not obtaining the desired result.
Re: csh - remove empty and duplicate unnecessary lines
Kalwe Caramalac wrote:
> Hi, this is my first diff submission, forgive me if have any error,
> if anyone has any tips on how to do this i appreciate it.
> @@ -588,7 +585,6 @@ dopopd(Char **v, struct command *t)
> void
> dfree(struct directory *dp)
> {
> -
> if (dp->di_count != 0) {
It's very common style to leave a blank line in functions without local
variables.
Also, your mailer has mangled the diff.
Re: apmd: zap useless globals
Klemens Nanni wrote: > Initialize stack variables directly instead of using global state in > between. > > OK? sure
Re: grep -ob: behave like ggrep, adapt documentation
Christopher Zimmermann wrote:
> Hi,
>
> I just tried to find the byte position of a string within a binary file
> using grep. Our base grep -bo let me down because it will only display
> the position of the '\n' delimited line, not the position of the
> pattern.
>
> That's what our grep(1) says:
> -bThe offset in bytes of a matched pattern is displayed in
> front of the respective matched line.
>
> what it actually does is displaying the offset of the line, not the
> pattern. I think it should read as following:
>
> -bEach output line is preceded by its position (in bytes) in the
> file. If option -o is also specified, the offset in bytes of
> the actual matched pattern is displayed.
This seems like the right behavior. But the diff can be simpler?
> Index: util.c
> ===
> RCS file: /cvs/src/usr.bin/grep/util.c,v
> retrieving revision 1.59
> diff -u -p -r1.59 util.c
> --- util.c23 Jan 2019 23:00:54 - 1.59
> +++ util.c16 Jul 2019 21:16:18 -
> @@ -250,9 +250,9 @@ print:
> if (Bflag > 0)
> printqueue();
> linesqueued = 0;
> - printline(l, ':', oflag ? : NULL);
> + printline(l, ':', );
> } else {
> - printline(l, '-', oflag ? : NULL);
> + printline(l, '-', );
> tail--;
> }
> }
> @@ -651,12 +651,14 @@ printline(str_t *line, int sep, regmatch
> if (bflag) {
> if (n)
> putchar(sep);
> - printf("%lld", (long long)line->off);
> + printf("%lld",
> + (long long)line->off
> + + (pmatch && oflag ? pmatch->rm_so : 0));
> ++n;
> }
> if (n)
> putchar(sep);
> - if (pmatch)
> + if (oflag && pmatch)
> fwrite(line->dat + pmatch->rm_so,
> pmatch->rm_eo - pmatch->rm_so, 1, stdout);
> else
I don't see why you needed to change this much.
- printf("%lld", (long long)line->off);
+ printf("%lld",
+ (long long)line->off
+ + (pmatch ? pmatch->rm_so : 0));
That should be enough, no?
more doas.1 tweaks
I think this wording clarifies what's happening.
1. Start by talking about creating a new environment. That's what we always
do. Everything afterwards is an operation performed on this new environment.
2. Move the list of magic variables out of doas.conf. I think it's better to
document this in one place. Note that setenv comes after everything else.
3. Add DOAS_USER to the list of variables set.
Index: doas.1
===
RCS file: /cvs/src/usr.bin/doas/doas.1,v
retrieving revision 1.21
diff -u -p -r1.21 doas.1
--- doas.1 19 Jun 2019 09:50:13 - 1.21
+++ doas.1 21 Jun 2019 16:46:28 -
@@ -40,7 +40,7 @@ or
.Fl s
is specified.
.Pp
-By default, the environment is reset.
+By default, a new environment is created.
The variables
.Ev HOME ,
.Ev LOGNAME ,
@@ -51,6 +51,9 @@ and
and the
.Xr umask 2
are set to values appropriate for the target user.
+.Ev DOAS_USER
+is set to the name of the user executing
+.Nm .
The variables
.Ev DISPLAY
and
Index: doas.conf.5
===
RCS file: /cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.38
diff -u -p -r1.38 doas.conf.5
--- doas.conf.5 19 Jun 2019 09:55:55 - 1.38
+++ doas.conf.5 21 Jun 2019 16:46:28 -
@@ -49,22 +49,11 @@ The user is not required to enter a pass
After the user successfully authenticates, do not ask for a password
again for some time.
.It Ic keepenv
-The user's environment is maintained.
-The default is to retain the variables
-.Ev DISPLAY
-and
-.Ev TERM
-from the invoking process, reset
-.Ev HOME ,
-.Ev LOGNAME ,
-.Ev PATH ,
-.Ev SHELL ,
-and
-.Ev USER
-as appropriate for the target user, and discard the rest of the environment.
+Environment variables other than those listed in
+.Xr doas 1
+are retained when creating the environment for the new process.
.It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic }
-In addition to the variables mentioned above, keep the space-separated
-specified variables.
+Keep or set the space-separated specified variables.
Variables may also be removed with a leading
.Sq -
or set using the latter syntax.
@@ -74,6 +63,7 @@ is a
.Ql $
then the value to be set is taken from the existing environment
variable of the indicated name.
+This option is processed after the default environment has been created.
.El
.It Ar identity
The username to match.
doas environmental security
I've made some changes to how doas handles environment variables recently. The diffs were in previous emails, and have been committed. Thanks to Sander Bos for pointing out some particular edge cases with the old handling. There are two (or more) ways to run doas. In the first, you use it to run some commands as root. I use doas to run ifconfig to adjust the network on my laptop. I also know the root password, but doas is a bit quicker and easier. Either way, I'm fully trusted. Another way to use doas is to setup a restricted root setting for an untrusted user. This is treading on thin ice, in my opinion, since there may be ways for the untrusted user to escape. The unix security model isn't very good at letting somebody be root, but only kinda sorta. Which brings us to environment variables. doas (previously) allowed the new process to inherit HOME and PATH (among other variables) from the invoking user. This reflected my attitude that, yeah, I'm running this program as another user, but I'm still me. This however, causes trouble where if you try to set up a restricted root setting, an evil doer can specify their own PATH and possibly get a shell script to exec something. I think people knew this, but maybe didn't pull on the string to see how far it goes? So Sander looked into this, and discovered even a simple command like less is potentially unsafe to run as restricted root. less will execute commands if specified in the LESSOPEN environemnt variable, however even if that's not set, it will open $HOME/.less and allow setting it from there. If you study the manual sufficiently, it's all in there. (Setting LESSSECURE would have been the safe thing if you wanted to restrict a user to just less, but I think the general point remains that programs read all sorts of files, and everything needs to exec everything else.) After some reflection, I've been convinced that it's unlikely everybody reads the manuals, or that the manuals are even correct or complete. So the new doas behavior moving forward is to reset most everything to the target user's environment. Your action items, as we like to say in the biz, are: 1. Check existing configs for "restricted root" rules and verify that they are run with the correct environment. 2. When updating, check for rules that intentionally use inherited environment variables. They may need to be explicitly passing using setenv in doas.conf.
doas fix PATH inheritance
espie found a bug. we reset PATH in the parent too soon, and then it's not
possible to change it back via setenv.
instead of trying to move code around, save a copy of the path before we mess
with it and make it available later. this lets setenv { PATH=$PATH } work.
Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.77
diff -u -p -r1.77 doas.c
--- doas.c 16 Jun 2019 18:16:34 - 1.77
+++ doas.c 17 Jun 2019 19:06:14 -
@@ -286,6 +286,7 @@ main(int argc, char **argv)
const char *confpath = NULL;
char *shargv[] = { NULL, NULL };
char *sh;
+ const char *p;
const char *cmd;
char cmdline[LINE_MAX];
char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
@@ -401,6 +402,11 @@ main(int argc, char **argv)
authuser(mypw->pw_name, login_style, rule->options & PERSIST);
}
+
+ if ((p = getenv("PATH")) != NULL)
+ formerpath = strdup(p);
+ if (formerpath == NULL)
+ formerpath = "";
if (unveil(_PATH_LOGIN_CONF, "r") == -1)
err(1, "unveil");
Index: doas.h
===
RCS file: /cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.14
diff -u -p -r1.14 doas.h
--- doas.h 16 Jun 2019 18:16:34 - 1.14
+++ doas.h 17 Jun 2019 19:06:14 -
@@ -29,6 +29,8 @@ extern struct rule **rules;
extern int nrules;
extern int parse_errors;
+extern const char *formerpath;
+
struct passwd;
char **prepenv(const struct rule *, const struct passwd *,
Index: env.c
===
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.8
diff -u -p -r1.8 env.c
--- env.c 17 Jun 2019 16:01:26 - 1.8
+++ env.c 17 Jun 2019 19:06:14 -
@@ -28,6 +28,8 @@
#include "doas.h"
+const char *formerpath;
+
struct envnode {
RB_ENTRY(envnode) node;
const char *key;
@@ -198,8 +200,12 @@ fillenv(struct env *env, const char **en
/* assign value or inherit from environ */
if (eq) {
val = eq + 1;
- if (*val == '$')
- val = getenv(val + 1);
+ if (*val == '$') {
+ if (strcmp(val + 1, "PATH") == 0)
+ val = formerpath;
+ else
+ val = getenv(val + 1);
+ }
} else {
val = getenv(name);
}
Re: new env defaults for doas
Todd C. Miller wrote: > On Mon, 17 Jun 2019 12:58:00 -0400, "Ted Unangst" wrote: > > > Committed this. I'm not entirely happy with the wording. I think hiding them > > under an option in the config man page is the wrong place. The default > > behavior should be documented in a more default place. > > I would just place that bit either before the options are described > or after the options in an "Execution environment" sub-section. Not sure what you mean. This diff does put it before the options. same place as su, although su has other problems. fortunately, we don't have the same combination of possibilities as su.
Re: new env defaults for doas
Ted Unangst wrote: > Yes, I think it's better to always reset these things. Here's a diff. > > 1. Always set HOME, PATH, SHELL etc to the target. Committed this. I'm not entirely happy with the wording. I think hiding them under an option in the config man page is the wrong place. The default behavior should be documented in a more default place. Also mention working directory is not changed. Index: doas.1 === RCS file: /cvs/src/usr.bin/doas/doas.1,v retrieving revision 1.19 diff -u -p -r1.19 doas.1 --- doas.1 4 Sep 2016 15:20:37 - 1.19 +++ doas.1 17 Jun 2019 16:57:26 - @@ -40,6 +40,23 @@ or .Fl s is specified. .Pp +By default, the environment is reset. +The variables +.Ev HOME , +.Ev LOGNAME , +.Ev PATH , +.Ev SHELL , +and +.Ev USER +are set to values appropriate for the target user. +The variables +.Ev DISPLAY +and +.Ev TERM +are inherited from the current environment. +This behavior may be modified by the config file. +The working directory is not changed. +.Pp The options are as follows: .Bl -tag -width tenletters .It Fl a Ar style
Re: new env defaults for doas
Martijn van Duren wrote:
> > 2. When doing keepenv, nothing changes, except addition of above.
>
> It feels inconsistent to make keepenv behave different here.
> - It may allow certain applications to behave unexpected compared to
> without keepenv (e.g. scripts that use $HOME/.cache).
> - The values are hard to retrofit into doas.conf by the end-user, while
> it's easy to keep the original with setenv { HOME ... }.
Yes, I think it's better to always reset these things. Here's a diff.
1. Always set HOME, PATH, SHELL etc to the target.
2. Without keepenv, other environment variables are discarded.
3. With keepenv, other variables are retained, but the above are still reset.
4. Possible to override this with setenv.
This is much more consistent and predictable.
Index: doas.conf.5
===
RCS file: /cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.36
diff -u -p -r1.36 doas.conf.5
--- doas.conf.5 16 Jun 2019 18:16:34 - 1.36
+++ doas.conf.5 16 Jun 2019 18:20:20 -
@@ -54,6 +54,14 @@ The default is to reset the environment,
.Ev DISPLAY
and
.Ev TERM .
+The variables
+.Ev HOME ,
+.Ev LOGNAME ,
+.Ev PATH ,
+.Ev SHELL ,
+and
+.Ev USER
+are always reset.
.It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic }
In addition to the variables mentioned above, keep the space-separated
specified variables.
Index: env.c
===
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.7
diff -u -p -r1.7 env.c
--- env.c 16 Jun 2019 18:16:34 - 1.7
+++ env.c 16 Jun 2019 18:20:20 -
@@ -85,6 +85,10 @@ static struct env *
createenv(const struct rule *rule, const struct passwd *mypw,
const struct passwd *targpw)
{
+ static const char *copyset[] = {
+ "DISPLAY", "TERM",
+ NULL
+ };
struct env *env;
u_int i;
@@ -95,6 +99,13 @@ createenv(const struct rule *rule, const
env->count = 0;
addnode(env, "DOAS_USER", mypw->pw_name);
+ addnode(env, "HOME", targpw->pw_dir);
+ addnode(env, "LOGNAME", targpw->pw_name);
+ addnode(env, "PATH", getenv("PATH"));
+ addnode(env, "SHELL", targpw->pw_shell);
+ addnode(env, "USER", targpw->pw_name);
+
+ fillenv(env, copyset);
if (rule->options & KEEPENV) {
extern const char **environ;
@@ -124,19 +135,6 @@ createenv(const struct rule *rule, const
env->count++;
}
}
- } else {
- static const char *copyset[] = {
- "DISPLAY", "TERM",
- NULL
- };
-
- addnode(env, "HOME", targpw->pw_dir);
- addnode(env, "LOGNAME", targpw->pw_name);
- addnode(env, "PATH", getenv("PATH"));
- addnode(env, "SHELL", targpw->pw_shell);
- addnode(env, "USER", targpw->pw_name);
-
- fillenv(env, copyset);
}
return env;
Re: new env defaults for doas
Martijn van Duren wrote: > I'm not convinced that LOGIN_SETPATH is a good idea here. From what I > gathered that sets PATH from login.conf(5), while most environments I > know will use .profile to set it and could cause unexpected behaviour > if the my and targ PATH are reset to unexpected values. > > I would vote to safe PATH from the caller instead of getting a likely > unexpected or incomplete PATH environment based on login.conf. Short answer is this is what su - does. We can set a default safe path, but there's no reason that's better than what's in .profile either. With login.conf, we get a safe default and a way for the admin to fix it if it's not what they want.
new env defaults for doas
This has come up a few times before. For background, the default rule for doas
is to copy a few environment settings from the user and omit the rest. This is
to prevent confusion, and also supposedly for security. However, some of the
alleged safe variables like PATH probably aren't that safe. And things like
USER are confusing? And why even bother with MAIL? The list is kinda adhoc and
mostly copied from what I understood sudo to do at the time, but I believe
sudo has changed defaults as well.
So here's a new model which I think is safer and more consistent.
1. Always add a DOAS_USER with the invoking user's name.
2. When doing keepenv, nothing changes, except addition of above.
3. When starting with a new environment, fill in USER and HOME and such with
the values of the target user, instead of copying from the invoking user. You
run a command as a user, you should have that user's environment.
4. DISPLAY and TERM are still copied, since they represent a description of
the actual environment in which we are running. (Not sure how useful display
is without xauth cookies, but not my problem.)
5. As before, anything can be overridden with setenv in the config.
This is a kinda breaking change, but probably in a good way.
For simple configurations, I expect nothing changes. For more complicated
setups, this new handling is probably closer to what the admin expects or
desires.
Index: doas.c
===
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.76
diff -u -p -r1.76 doas.c
--- doas.c 12 Jun 2019 02:50:29 - 1.76
+++ doas.c 13 Jun 2019 02:11:07 -
@@ -421,6 +421,7 @@ main(int argc, char **argv)
errx(1, "no passwd entry for target");
if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
+ LOGIN_SETPATH |
LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
LOGIN_SETUSER) != 0)
errx(1, "failed to set user context for target");
@@ -439,8 +440,13 @@ main(int argc, char **argv)
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
mypw->pw_name, cmdline, targpw->pw_name, cwd);
- envp = prepenv(rule);
+ envp = prepenv(rule, mypw, targpw);
+ if (rule->cmd) {
+ /* do this again after setusercontext reset it */
+ if (setenv("PATH", safepath, 1) == -1)
+ err(1, "failed to set PATH '%s'", safepath);
+ }
execvpe(cmd, argv, envp);
fail:
if (errno == ENOENT)
Index: doas.conf.5
===
RCS file: /home/cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.35
diff -u -p -r1.35 doas.conf.5
--- doas.conf.5 7 Feb 2018 05:13:57 - 1.35
+++ doas.conf.5 13 Jun 2019 01:41:18 -
@@ -51,15 +51,9 @@ again for some time.
.It Ic keepenv
The user's environment is maintained.
The default is to reset the environment, except for the variables
-.Ev DISPLAY ,
-.Ev HOME ,
-.Ev LOGNAME ,
-.Ev MAIL ,
-.Ev PATH ,
-.Ev TERM ,
-.Ev USER
+.Ev DISPLAY
and
-.Ev USERNAME .
+.Ev TERM .
.It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic }
In addition to the variables mentioned above, keep the space-separated
specified variables.
Index: doas.h
===
RCS file: /home/cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.13
diff -u -p -r1.13 doas.h
--- doas.h 6 Apr 2017 21:12:06 - 1.13
+++ doas.h 13 Jun 2019 01:10:29 -
@@ -29,7 +29,10 @@ extern struct rule **rules;
extern int nrules;
extern int parse_errors;
-char **prepenv(const struct rule *);
+struct passwd;
+
+char **prepenv(const struct rule *, const struct passwd *,
+const struct passwd *);
#define PERMIT 1
#define DENY 2
Index: env.c
===
RCS file: /home/cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.6
diff -u -p -r1.6 env.c
--- env.c 6 Apr 2017 21:12:06 - 1.6
+++ env.c 13 Jun 2019 02:12:57 -
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include "doas.h"
@@ -38,6 +39,8 @@ struct env {
u_int count;
};
+static void fillenv(struct env *env, const char **envlist);
+
static int
envcmp(struct envnode *a, struct envnode *b)
{
@@ -68,8 +71,19 @@ freenode(struct envnode *node)
free(node);
}
+static void
+addnode(struct env *env, const char *key, const char *value)
+{
+ struct envnode *node;
+
+ node = createnode(key, value);
+ RB_INSERT(envtree, >root, node);
+ env->count++;
+}
+
static struct env *
-createenv(const struct rule *rule)
+createenv(const struct rule *rule, const struct passwd *mypw,
+const struct passwd *targpw)
{
struct env *env;
u_int i;
@@ -80,6 +94,8 @@ createenv(const struct rule *rule)
Re: fsync(2) and I/O errors
Maximilian Lorlacks wrote: > This looks okay to me. > > (plus two months ping) oh, good news, committed two months ago. sorry, forgot to reply. > > ‐‐‐ Original Message ‐‐‐ > On Tuesday, April 16, 2019 8:19 PM, Ted Unangst wrote: > > > Oh, right, I reworded it slightly, but I think this is something we should > > note. > > > > Index: fsync.2 > > > > = > > > > RCS file: /home/cvs/src/lib/libc/sys/fsync.2,v > > retrieving revision 1.14 > > diff -u -p -r1.14 fsync.2 > > --- fsync.2 10 Sep 2015 17:55:21 - 1.14 > > +++ fsync.2 16 Apr 2019 20:18:03 - > > @@ -66,6 +66,16 @@ and > > .Fn fdatasync > > should be used by programs that require a file to be in a known state, > > for example, in building a simple transaction facility. > > +.Pp > > +If > > +.Fn fsync > > +or > > +.Fn fdatasync > > +fails with > > +.Er EIO , > > +the state of the on-disk data may have been only partially written. > > +To guard against potential inconsistency, future calls will continue > > failing > > +until all references to the file are closed. > > .Sh RETURN VALUES > > .Rv -std fsync fdatasync > > .Sh ERRORS > >
Re: use getpwuid_r in doas
Ted Unangst wrote:
> I have a coming change which will need to access both the calling user and
> target users' passwd entries. In order to accomplish this, we need to switch
> to the reentrant flavor of getpwuid. No behaviorial change, but I think this
> is clearer and less error prone as well, versus reusing a pointer to static
> storage.
Improve a few more things noticed by martijn. We don't need to copy strings
that have long lifetimes. Also, fix some error checks to be more precise.
Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.75
diff -u -p -r1.75 doas.c
--- doas.c 10 Jun 2019 18:11:27 - 1.75
+++ doas.c 10 Jun 2019 18:15:34 -
@@ -288,7 +288,6 @@ main(int argc, char **argv)
char *sh;
const char *cmd;
char cmdline[LINE_MAX];
- char myname[_PW_NAME_LEN + 1];
char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
struct passwd mypwstore, targpwstore;
struct passwd *mypw, *targpw;
@@ -349,10 +348,10 @@ main(int argc, char **argv)
usage();
rv = getpwuid_r(uid, , mypwbuf, sizeof(mypwbuf), );
- if (rv != 0 || mypw == NULL)
+ if (rv != 0)
err(1, "getpwuid_r failed");
- if (strlcpy(myname, mypw->pw_name, sizeof(myname)) >= sizeof(myname))
- errx(1, "pw_name too long");
+ if (mypw == NULL)
+ errx(1, "no passwd entry for self");
ngroups = getgroups(NGROUPS_MAX, groups);
if (ngroups == -1)
err(1, "can't get groups");
@@ -361,9 +360,7 @@ main(int argc, char **argv)
if (sflag) {
sh = getenv("SHELL");
if (sh == NULL || *sh == '\0') {
- shargv[0] = strdup(mypw->pw_shell);
- if (shargv[0] == NULL)
- err(1, NULL);
+ shargv[0] = mypw->pw_shell;
} else
shargv[0] = sh;
argv = shargv;
@@ -394,7 +391,7 @@ main(int argc, char **argv)
if (!permit(uid, groups, ngroups, , target, cmd,
(const char **)argv + 1)) {
syslog(LOG_AUTHPRIV | LOG_NOTICE,
- "failed command for %s: %s", myname, cmdline);
+ "failed command for %s: %s", mypw->pw_name, cmdline);
errc(1, EPERM, NULL);
}
@@ -402,7 +399,7 @@ main(int argc, char **argv)
if (nflag)
errx(1, "Authorization required");
- authuser(myname, login_style, rule->options & PERSIST);
+ authuser(mypw->pw_name, login_style, rule->options & PERSIST);
}
if (unveil(_PATH_LOGIN_CONF, "r") == -1)
@@ -418,7 +415,9 @@ main(int argc, char **argv)
err(1, "pledge");
rv = getpwuid_r(target, , targpwbuf, sizeof(targpwbuf),
);
- if (rv != 0 || targpw == NULL)
+ if (rv != 0)
+ err(1, "getpwuid_r failed");
+ if (targpw == NULL)
errx(1, "no passwd entry for target");
if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
@@ -438,7 +437,7 @@ main(int argc, char **argv)
err(1, "pledge");
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
- myname, cmdline, targpw->pw_name, cwd);
+ mypw->pw_name, cmdline, targpw->pw_name, cwd);
envp = prepenv(rule);
use getpwuid_r in doas
I have a coming change which will need to access both the calling user and
target users' passwd entries. In order to accomplish this, we need to switch
to the reentrant flavor of getpwuid. No behaviorial change, but I think this
is clearer and less error prone as well, versus reusing a pointer to static
storage.
Index: doas.c
===
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.74
diff -u -p -r1.74 doas.c
--- doas.c 17 Jan 2019 05:35:35 - 1.74
+++ doas.c 21 May 2019 17:04:04 -
@@ -289,13 +289,15 @@ main(int argc, char **argv)
const char *cmd;
char cmdline[LINE_MAX];
char myname[_PW_NAME_LEN + 1];
- struct passwd *pw;
+ char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
+ struct passwd mypwstore, targpwstore;
+ struct passwd *mypw, *targpw;
const struct rule *rule;
uid_t uid;
uid_t target = 0;
gid_t groups[NGROUPS_MAX + 1];
int ngroups;
- int i, ch;
+ int i, ch, rv;
int sflag = 0;
int nflag = 0;
char cwdpath[PATH_MAX];
@@ -346,10 +348,10 @@ main(int argc, char **argv)
} else if ((!sflag && !argc) || (sflag && argc))
usage();
- pw = getpwuid(uid);
- if (!pw)
- err(1, "getpwuid failed");
- if (strlcpy(myname, pw->pw_name, sizeof(myname)) >= sizeof(myname))
+ rv = getpwuid_r(uid, , mypwbuf, sizeof(mypwbuf), );
+ if (rv != 0 || mypw == NULL)
+ err(1, "getpwuid_r failed");
+ if (strlcpy(myname, mypw->pw_name, sizeof(myname)) >= sizeof(myname))
errx(1, "pw_name too long");
ngroups = getgroups(NGROUPS_MAX, groups);
if (ngroups == -1)
@@ -359,7 +361,7 @@ main(int argc, char **argv)
if (sflag) {
sh = getenv("SHELL");
if (sh == NULL || *sh == '\0') {
- shargv[0] = strdup(pw->pw_shell);
+ shargv[0] = strdup(mypw->pw_shell);
if (shargv[0] == NULL)
err(1, NULL);
} else
@@ -415,11 +417,11 @@ main(int argc, char **argv)
if (pledge("stdio rpath getpw exec id", NULL) == -1)
err(1, "pledge");
- pw = getpwuid(target);
- if (!pw)
+ rv = getpwuid_r(target, , targpwbuf, sizeof(targpwbuf),
);
+ if (rv != 0 || targpw == NULL)
errx(1, "no passwd entry for target");
- if (setusercontext(NULL, pw, target, LOGIN_SETGROUP |
+ if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
LOGIN_SETUSER) != 0)
errx(1, "failed to set user context for target");
@@ -436,7 +438,7 @@ main(int argc, char **argv)
err(1, "pledge");
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
- myname, cmdline, pw->pw_name, cwd);
+ myname, cmdline, targpw->pw_name, cwd);
envp = prepenv(rule);
Re: snake: unveil + pledge earlier
Jake Champlin wrote:
> - readscores(1);
> penalty = loot = 0;
> initscr();
> + if (unveil(scorepath, "rwc") == -1)
> + err(1, "unveil");
> +#ifdef LOGGING
> + if (unveil(logpath, "rwc") == -1)
> + err(1, "unveil");
> + logfile = fopen(logpath, "a");
> +#endif
> + readscores(1);
>
> if (pledge("stdio tty", NULL) == -1)
> err(1, "pledge");
We're unveiling the same file we're going to immediately open. And then we
never open it later, as proved by pledge without rpath. There doesn't seem to
be any benefit in this case.
If we needed to reopen these files at some later point, this would be the
right thing.
Re: caesar(6) to accept negative argument
Otto Moerbeek wrote: > We're computing modulo 26 here. Negative numbers have a positive > equivalent. So you diff adds code for no benefit. I think the amount of code added is an acceptable cost for improved user experience. We could use this argument to remove subtraction from bc, but that would be silly.
Re: free() sizes for ahc(4)
Miod Vallat wrote: > Note ahc_set_name() gets invoked with the dv_xname field of a struct > device, so it's not a good idea to free anything, should it be invoked > more than once. nice catch.
Re: softraid(4): fix wrong malloc size and zero sized free calls
Jan Klemkow wrote: > Hi, > > The diff mainly add sizes to free(9) calls. But, while here fix a > malloc(9) call with the wrong size in sr_ioctl_installboot(). > omi->omi_som is allocated with size of struct sr_meta_crypto, but used > as struct sr_meta_boot later. > > One free(9) with size zero left over in sr_discipline_free(). As, the > allocated size of sd->sd_vol.sv_chunks is not ascertainable here without > larger changes. > > Build and run the kernel for testing. The change in installboot for the size is suspicious. I would leave that out. Especially if you haven't tested installboot to a crypto softraid. :)
Re: add newline for tpm0: dmesg line
f. holop wrote: > dmesg: > > ... > acpibtn2 at acpi0: PWRB > tpm0 at acpi0: TPM_ addr 0xfed4/0x5000acpivideo0 at acpi0: GFX0 > acpivout0 at acpivideo0: DD1F > ... thanks
Re: sv(4): fix free with zero size
Jan Klemkow wrote: > Hi, > > The following diff fixes "free with zero size" in sv(4). Builds and > stats the kernel with sv at pci and audio at sv enabled. ok
Re: chroot vs su vs doas
Martijn van Duren wrote: > >> Would > >> doas -c /rootdir somecmd > >> be of any use ? > > > > Not particularly opposed, but the extend of this option should be > > examined. E.g. do we want to extend it to the config to be something > > similar to -u and limit it's use for certain commands? > > > Working this out I'm not particularly a fan of the extra code this would > take, but the diff below seems to do the trick. This seems like feature creep. We already have a command to chroot and switch user.
Re: chroot vs su vs doas
Martijn van Duren wrote:
> > But what would it hurt to allow root usage ?
> > Specifically,
> >
> > doas -u ${BUILDUSER} some unquoted command
> >
> > as run by root. This would not open any security hole, would it ?
>
> I don't see any and I've been bitten by having a rootshell open and
> typing doas out of habit.
The reason there's no builtin config is to prevent confusion, even if it
sometimes causes mild annoyance. When there are invisible rules, it becomes
harder to know which rule is actually being taken.
For example, your rule below doesn't include keepenv. So next week we're going
to get bug reports that things work when run as a user, but not as root. And
for exactly the same reason, people only half set things up. The fact that the
default appears to work sometimes makes things even more annoying. And I
don't think the default should just be changed to include keepenv, because
maybe that's not what people want and then we'd need to explain how to undo
that.
> + static struct rule allowroot = {
> + .action = PERMIT,
> + .options = NOPASS,
> + .ident = NULL,
> + .target = NULL,
> + .cmd = NULL,
> + .cmdargs = NULL,
> + .envlist = NULL
> + };
Re: cmdfile for config -ef
Benjamin Baier wrote: > On Sat, 11 May 2019 11:18:02 -0400 > "Ted Unangst" wrote: > > > This adds a new option to config, -c cmdfile, that allows reading commands > > from a file instead of stdin. This makes it easier to script. > > Interesting. Can the cmdfile be /bsd ? > So something like config -u but without the need of /dev/mem ? No, the intended use case is not to copy options between kernels, but to apply a consistent set of changes after linking.
cmdfile for config -ef
This adds a new option to config, -c cmdfile, that allows reading commands
from a file instead of stdin. This makes it easier to script.
Index: config.8
===
RCS file: /home/cvs/src/usr.sbin/config/config.8,v
retrieving revision 1.66
diff -u -p -r1.66 config.8
--- config.825 Apr 2018 12:01:11 - 1.66
+++ config.811 May 2019 15:16:45 -
@@ -43,6 +43,7 @@
.Op Fl s Ar srcdir
.Op Ar config-file
.Nm config
+.Op Fl c Ar cmdfile
.Op Fl u
.Op Fl f | o Ar outfile
.Fl e
@@ -103,6 +104,9 @@ directories above the build directory).
.Pp
For kernel modification, the options are as follows:
.Bl -tag -width Ds
+.It Fl c Ar cmdfile
+Read commands from the specified file instead of the standard input.
+Save and quit automatically when the end of file is reached.
.It Fl e
Allows the modification of kernel device configuration (see
.Xr boot_config 8 ) .
Index: main.c
===
RCS file: /home/cvs/src/usr.sbin/config/main.c,v
retrieving revision 1.59
diff -u -p -r1.59 main.c
--- main.c 22 Jun 2017 15:57:16 - 1.59
+++ main.c 11 May 2019 15:00:34 -
@@ -82,7 +82,7 @@ usage(void)
fprintf(stderr,
"usage: %s [-p] [-b builddir] [-s srcdir] [config-file]\n"
- " %s [-u] [-f | -o outfile] -e infile\n",
+ " %s [-c cmdfile] [-u] [-f | -o outfile] -e infile\n",
__progname, __progname);
exit(1);
@@ -92,6 +92,7 @@ int pflag = 0;
char *sflag = NULL;
char *bflag = NULL;
char *startdir;
+char *cmdfile;
int
main(int argc, char *argv[])
@@ -105,9 +106,11 @@ main(int argc, char *argv[])
err(1, "pledge");
pflag = eflag = uflag = fflag = 0;
- while ((ch = getopt(argc, argv, "epfb:s:o:u")) != -1) {
+ while ((ch = getopt(argc, argv, "c:epfb:s:o:u")) != -1) {
switch (ch) {
-
+ case 'c':
+ cmdfile = optarg;
+ break;
case 'o':
outfile = optarg;
break;
Index: misc.c
===
RCS file: /home/cvs/src/usr.sbin/config/misc.c,v
retrieving revision 1.9
diff -u -p -r1.9 misc.c
--- misc.c 2 Oct 2011 22:20:49 - 1.9
+++ misc.c 11 May 2019 15:08:56 -
@@ -38,13 +38,10 @@
extern int verbose;
int
-ask_cmd(cmd_t *cmd)
+parse_cmd(cmd_t *cmd, char *lbuf)
{
- char lbuf[100], *cp, *buf;
+ char *cp, *buf;
- /* Get input */
- if (fgets(lbuf, sizeof lbuf, stdin) == NULL)
- errx(1, "eof");
lbuf[strcspn(lbuf, "\n")] = '\0';
if (verbose)
printf("%s\n", lbuf);
@@ -59,6 +56,17 @@ ask_cmd(cmd_t *cmd)
strlcpy(cmd->args, buf, sizeof cmd->args);
return (0);
+}
+
+int
+ask_cmd(cmd_t *cmd)
+{
+ char lbuf[100];
+
+ /* Get input */
+ if (fgets(lbuf, sizeof lbuf, stdin) == NULL)
+ errx(1, "eof");
+ return parse_cmd(cmd, lbuf);
}
int
Index: misc.h
===
RCS file: /home/cvs/src/usr.sbin/config/misc.h,v
retrieving revision 1.4
diff -u -p -r1.4 misc.h
--- misc.h 3 Jun 2003 00:52:35 - 1.4
+++ misc.h 11 May 2019 15:10:04 -
@@ -33,6 +33,7 @@
/* Prototypes */
int ask_cmd(cmd_t *);
+int parse_cmd(cmd_t *, char *);
int ask_yn(const char *);
#endif /* _MISC_H */
Index: ukcutil.c
===
RCS file: /home/cvs/src/usr.sbin/config/ukcutil.c,v
retrieving revision 1.23
diff -u -p -r1.23 ukcutil.c
--- ukcutil.c 27 Sep 2017 15:14:52 - 1.23
+++ ukcutil.c 11 May 2019 15:14:25 -
@@ -29,6 +29,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1295,13 +1296,61 @@ add_history(int devno, short unit, short
}
int
+config_fromfile(const char *cmdfile) {
+ FILE *fp;
+ cmd_t cmd;
+ int i;
+
+ fp = fopen(cmdfile, "r");
+ if (fp == NULL)
+ err(1, "open %s", cmdfile);
+
+ /* Set up command table pointer */
+ cmd.table = cmd_table;
+
+ /* Edit cycle */
+ do {
+ char lbuf[100];
+
+ /* Get input */
+ if (fgets(lbuf, sizeof lbuf, fp) == NULL)
+ break;
+ parse_cmd(, lbuf);
+
+ if (cmd.cmd[0] == '\0')
+ continue;
+ for (i = 0; cmd_table[i].cmd != NULL; i++)
+ if (strstr(cmd_table[i].cmd, cmd.cmd) ==
+ cmd_table[i].cmd)
+ break;
+
+ /* Check for valid command */
+ if (cmd_table[i].cmd == NULL) {
+ printf("Invalid command '%s'\n", cmd.cmd);
+
Re: [patch] improve strptime(3) %z timezone parsing
Ingo Schwarze wrote: > Ouch. No, it does not. Thanks for spotting the regression. > > The following patch preserves the parsing behaviour > and correctly stores the number of seconds into tm_gmtoff. oops, missed that case too. this looks correct.
Re: [patch] improve strptime(3) %z timezone parsing
Ingo Schwarze wrote: > Now let's get to the more serious part. > Hiltjo observed that %Z and %z produce wrong results. > > First of all, neither POSIX nor XPG define tm_gmtoff nor %Z nor %z: > > https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.html > https://pubs.opengroup.org/onlinepubs/9699919799/functions/strptime.html > > So i think the best way to find out what tm_gmtoff should be is to > understand how programs in our own tree use it. > > Here is a (hopefully) complete list of the users in OpenBSD base. > The following files expect it to contain seconds: > - lib/libc/time/strftime.c > - lib/libc/time/wcsftime.c > - usr.sbin/smtpd/to.c > - usr.sbin/snmpd/mib.c > - usr.sbin/cron/cron.c > - usr.bin/ftp/util.c > - usr.bin/cvs/entries.c > - usr.bin/rcs/rcstime.c > - gnu/usr.bin/perl/time64.c > > I failed to find any users that do *not* expect seconds. > So my conclusion is that the documentation is right and > what the code in strptime.c does is wrong. > > Here is a patch to fix the code. this looks good to me. ok.
Re: ssl(8), fix text about web browsers and SAN
Stuart Henderson wrote: > it's standard behaviour for web browsers to not use hostnames in > Subject at all but require SAN. current ssl(8) text suggests "some new" > and "deprecated" rather than "stopped supporting". > > comments/ok? I was trying to avoid argument "but my browser still works" :) but I agree this wording is closer to reality. ok.
Re: [patch] improve strptime(3) %z timezone parsing
Ingo Schwarze wrote: > I'm not mixing anything else into this diff. The other bugs should > be handled separately. > > OK? Works for me. (with additional comment removal)
