Re: patch unveil fail
On Wed, 25 Oct 2023 13:38:37 +0200, Alexander Bluhm wrote: > Since 7.4 patch(1) does not work if an explicit patchfile is given on > command line. > > https://marc.info/?l=openbsd-cvs=168941770509379=2 OK millert@ - todd
Re: timeout(1): align execvp(3) failure statuses with GNU timeout
On Mon, 16 Oct 2023 10:33:18 -0500, Scott Cheloha wrote: > Per deraadt@, update EXIT STATUS, too. Looks good to me. > Do we need to keep the "128 + signal" bit? I thought that was a > normal Unix thing, and we tend to avoid saying things that are > implicitly true for every utility. I could go either way on that. It's true that this is default shell behavior so you probably don't need to document it. - todd
Re: timeout(1): align execvp(3) failure statuses with GNU timeout
On Sun, 15 Oct 2023 13:53:46 -0500, Scott Cheloha wrote: > Align timeout(1)'s execvp(3) failure statuses with those of GNU > timeout. 127 for ENOENT, 126 for everything else. Looks correct to me. OK millert@ - todd
Re: CVS: cvs.openbsd.org: src
On Tue, 10 Oct 2023 10:14:10 -0700, Chris Cappuccio wrote: > The Message-ID should be added to any message that doesn't have one. > An existing Message-ID should not be removed or changed. > > The RFC says it "MAY be applied when necessary by an originating SMTP server" > so the port numbers aren't a terrible idea, but it leaves open plenty > of room to not add one if someone isn't following the 465/587 scheme. But smtpd may not be the originating SMTP server. For "local" messages (that originated on the server) it knows to add the Message-ID if missing. I don't think it should be adding it to relayed messages. Messages received on the submission port are special, they need to be treated as local even though they originated elsewhere. - todd
Re: CVS: cvs.openbsd.org: src
On Tue, 10 Oct 2023 10:50:08 +0100, Stuart Henderson wrote: > Presumably 465 should be treated the same, though the hardcoded ports > don't feel entirely right here - this is presumably something that would > want adding for any connection which is allowed to relay .. Yes, I think so. Hard-coding ports is not great but there isn't a way in the config file to indicate that explicitly. - todd
Re: missing FreeBSD stdio patch
On Wed, 04 Oct 2023 11:53:51 -0600, Todd C. Miller wrote:
> Yes, this should be fixed. One difference is that in FreeBSD,
> __swsetup() sets errno whereas in OpenBSD we set errno in the caller
> when cantwrite() is true. I think it makes sense to set both errno
> and the __SERR flag in the same place. We just need to decide which
> place that is :-)
I decided that it is simplest to set errno and __SERR in __swsetup()
like FreeBSD has done instead of in the callers. This is consistent
with how fread(3) relies on __srefill() to set both errno and the
error flag.
We have an additional check in __swsetup() where we return EOF on
error that also needs to set errno and __SERR. There is no actual
fd in that case so I've set errno to EINVAL.
- todd
Index: lib/libc/stdio/fvwrite.c
===
RCS file: /cvs/src/lib/libc/stdio/fvwrite.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 fvwrite.c
--- lib/libc/stdio/fvwrite.c17 Mar 2017 16:06:33 - 1.20
+++ lib/libc/stdio/fvwrite.c4 Oct 2023 18:28:33 -
@@ -34,7 +34,6 @@
#include
#include
#include
-#include
#include
#include "local.h"
#include "fvwrite.h"
@@ -58,10 +57,8 @@ __sfvwrite(FILE *fp, struct __suio *uio)
if ((len = uio->uio_resid) == 0)
return (0);
/* make sure we can write */
- if (cantwrite(fp)) {
- errno = EBADF;
+ if (cantwrite(fp))
return (EOF);
- }
#defineMIN(a, b) ((a) < (b) ? (a) : (b))
#defineCOPY(n) (void)memcpy(fp->_p, p, n)
Index: lib/libc/stdio/putc.c
===
RCS file: /cvs/src/lib/libc/stdio/putc.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 putc.c
--- lib/libc/stdio/putc.c 31 Aug 2015 02:53:57 - 1.13
+++ lib/libc/stdio/putc.c 4 Oct 2023 18:28:37 -
@@ -32,7 +32,6 @@
*/
#include
-#include
#include "local.h"
/*
@@ -43,10 +42,8 @@
int
putc_unlocked(int c, FILE *fp)
{
- if (cantwrite(fp)) {
- errno = EBADF;
+ if (cantwrite(fp))
return (EOF);
- }
_SET_ORIENTATION(fp, -1);
return (__sputc(c, fp));
}
Index: lib/libc/stdio/vfprintf.c
===
RCS file: /cvs/src/lib/libc/stdio/vfprintf.c,v
retrieving revision 1.81
diff -u -p -u -r1.81 vfprintf.c
--- lib/libc/stdio/vfprintf.c 8 Sep 2021 15:57:27 - 1.81
+++ lib/libc/stdio/vfprintf.c 4 Oct 2023 16:40:18 -
@@ -457,10 +457,8 @@ __vfprintf(FILE *fp, const char *fmt0, _
_SET_ORIENTATION(fp, -1);
/* sorry, fprintf(read_only_file, "") returns EOF, not 0 */
- if (cantwrite(fp)) {
- errno = EBADF;
+ if (cantwrite(fp))
return (EOF);
- }
/* optimise fprintf(stderr) (and other unbuffered Unix files) */
if ((fp->_flags & (__SNBF|__SWR|__SRW)) == (__SNBF|__SWR) &&
Index: lib/libc/stdio/vfwprintf.c
===
RCS file: /cvs/src/lib/libc/stdio/vfwprintf.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 vfwprintf.c
--- lib/libc/stdio/vfwprintf.c 8 Sep 2021 15:57:27 - 1.22
+++ lib/libc/stdio/vfwprintf.c 4 Oct 2023 16:40:18 -
@@ -451,10 +451,8 @@ __vfwprintf(FILE * __restrict fp, const
_SET_ORIENTATION(fp, 1);
/* sorry, fwprintf(read_only_file, "") returns EOF, not 0 */
- if (cantwrite(fp)) {
- errno = EBADF;
+ if (cantwrite(fp))
return (EOF);
- }
/* optimise fwprintf(stderr) (and other unbuffered Unix files) */
if ((fp->_flags & (__SNBF|__SWR|__SRW)) == (__SNBF|__SWR) &&
Index: lib/libc/stdio/wbuf.c
===
RCS file: /cvs/src/lib/libc/stdio/wbuf.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 wbuf.c
--- lib/libc/stdio/wbuf.c 31 Aug 2015 02:53:57 - 1.13
+++ lib/libc/stdio/wbuf.c 4 Oct 2023 18:28:45 -
@@ -32,7 +32,6 @@
*/
#include
-#include
#include "local.h"
/*
@@ -54,10 +53,8 @@ __swbuf(int c, FILE *fp)
* calls might wrap _w from negative to positive.
*/
fp->_w = fp->_lbfsize;
- if (cantwrite(fp)) {
- errno = EBADF;
+ if (cantwrite(fp))
return (EOF);
- }
c = (unsigned char)c;
/*
Index: lib/libc/stdio/wsetup.c
===
RCS file: /cvs/src/lib/libc/stdio/wsetup.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 wsetup.c
--- lib/libc/stdio/wsetup.c 8 Aug 2005 08:05:36 - 1.7
+++ lib/libc/stdio/wsetup.c 4 Oct 2023 18:23:48 -
@@ -31,6 +31,7 @@
* SUCH DAMAGE.
*/
+#inc
Re: missing FreeBSD stdio patch
On Tue, 03 Oct 2023 16:09:12 -0700, enh wrote: > looks like OpenBSD is missing this patch from FreeBSD? llvm libc++ hit > this (a test worked on other OSes, including FreeBSD-based macOS) but > failed on OpenBSD-based Android. > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=127335 was the > original FreeBSD bug report. POSIX seems to explicitly require this > behavior (https://pubs.opengroup.org/onlinepubs/9699919799/functions/fwrite.h > tml: > "The fwrite() function shall return the number of elements > successfully written, which may be less than nitems if a write error > is encountered. If size or nitems is 0, fwrite() shall return 0 and > the state of the stream remains unchanged. Otherwise, if a write error > occurs, the error indicator for the stream shall be set, and errno > shall be set to indicate the error."). Yes, this should be fixed. One difference is that in FreeBSD, __swsetup() sets errno whereas in OpenBSD we set errno in the caller when cantwrite() is true. I think it makes sense to set both errno and the __SERR flag in the same place. We just need to decide which place that is :-) - todd
Re: Buffer overflow in /usr/bin/deroff
On Wed, 27 Sep 2023 10:59:26 -0600, "Todd C. Miller" wrote:
> I think we want support for arbitrary line lengths. There is only
> one place where we need to reallocate the line buffer.
The correct check is for "lp - line == linesz - 1". The code will
overwrite the newline with a NUL so we don't need to leave space
for it explicitly.
As written, deroff will not emit a line that does not end with a
newline. That could be changed in a subsequent commit if it so
desired.
- todd
Index: usr.bin/deroff/deroff.c
===
RCS file: /cvs/src/usr.bin/deroff/deroff.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 deroff.c
--- usr.bin/deroff/deroff.c 8 Mar 2023 04:43:10 - 1.17
+++ usr.bin/deroff/deroff.c 27 Sep 2023 19:54:20 -
@@ -135,7 +135,8 @@ int keepblock; /* keep blocks of text; n
char chars[128]; /* SPECIAL, PUNCT, APOS, DIGIT, or LETTER */
-char line[LINE_MAX];
+size_t linesz;
+char *line;
char *lp;
int c;
@@ -342,6 +343,10 @@ main(int ac, char **av)
files[0] = infile;
filesp = [0];
+ linesz = LINE_MAX;
+ if ((line = malloc(linesz)) == NULL)
+ err(1, NULL);
+
for (i = 'a'; i <= 'z'; ++i)
chars[i] = LETTER;
for (i = 'A'; i <= 'Z'; ++i)
@@ -477,7 +482,15 @@ regline(void (*pfunc)(char *, int), int
line[0] = c;
lp = line;
- while (lp - line < sizeof(line)) {
+ for (;;) {
+ if (lp - line == linesz - 1) {
+ char *newline = reallocarray(line, linesz, 2);
+ if (newline == NULL)
+ err(1, NULL);
+ lp = newline + (lp - line);
+ line = newline;
+ linesz *= 2;
+ }
if (c == '\\') {
*lp = ' ';
backsl();
Re: Buffer overflow in /usr/bin/deroff
On Wed, 27 Sep 2023 08:37:49 -0300, Crystal Kolipe wrote:
> So what do we want?
>
> 1. Traditional OpenBSD behaviour of breaking input lines at 2047,
>(which never actually worked correctly up to now).
> 2. Breaking input at 2048.
> 3. Support for arbitrary line length with no breaking.
>
> Presumably nobody is relying on the current behaviour in scripts or other
> code, as it's always been broken.
I think we want support for arbitrary line lengths. There is only
one place where we need to reallocate the line buffer.
- todd
Index: usr.bin/deroff/deroff.c
===
RCS file: /cvs/src/usr.bin/deroff/deroff.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 deroff.c
--- usr.bin/deroff/deroff.c 8 Mar 2023 04:43:10 - 1.17
+++ usr.bin/deroff/deroff.c 27 Sep 2023 16:56:54 -
@@ -135,7 +135,8 @@ int keepblock; /* keep blocks of text; n
char chars[128]; /* SPECIAL, PUNCT, APOS, DIGIT, or LETTER */
-char line[LINE_MAX];
+size_t linesz;
+char *line;
char *lp;
int c;
@@ -342,6 +343,10 @@ main(int ac, char **av)
files[0] = infile;
filesp = [0];
+ linesz = LINE_MAX;
+ if ((line = malloc(linesz)) == NULL)
+ err(1, NULL);
+
for (i = 'a'; i <= 'z'; ++i)
chars[i] = LETTER;
for (i = 'A'; i <= 'Z'; ++i)
@@ -477,7 +482,15 @@ regline(void (*pfunc)(char *, int), int
line[0] = c;
lp = line;
- while (lp - line < sizeof(line)) {
+ for (;;) {
+ if (lp - line == linesz - 2) {
+ char *newline = reallocarray(line, linesz, 2);
+ if (newline == NULL)
+ err(1, NULL);
+ lp = newline + (lp - line);
+ line = newline;
+ linesz *= 2;
+ }
if (c == '\\') {
*lp = ' ';
backsl();
Re: malloc write after free error checking
On Sun, 24 Sep 2023 09:58:53 +0200, Otto Moerbeek wrote: > The wayland issue was found as well, using the same method. > I'm working on programming the heuristic that is quite effective into > malloc itself. It currently looks like this for the X case above: > > X(67417) in malloc(): write to free mem 0xdd0277d4270 [0..7]@16 > allocated at /usr/lib/libc.so.97.1 0x92f22 > (preceding chunk 0xdd0277d4260 allocated at /usr/X11R6/bin/X 0x177374 (now fr > ee)) > > $ addr2line -e /usr/lib/libc.so.97.1 0x92f22 > /usr/src/lib/libc/string/strdup.c:45 > $ addr2line -e /usr/X11R6/bin/X 0x177374 > /usr/xenocara/xserver/Xext/xvdisp.c:995 > > The idea is: if a buffer overflow is detected, it is very wel possible > that the overwrite happened as an out-of-bound write of the preceding > chunk. It is also possible that it is a genuine write-after-free, in > that case the developer should inspect the code that allocated and > manipulated the first chunk. Malloc has no way to the the difference, > that requires debugging by a human. > > This is all experimental and the final form may change but it > certainly look promising, especially as regular malloc code did not > change at all (the extra info needed is only collected if malloc flag > D is set). This is very cool. Being able to tell where the (now-freed) chunk was allocated is a huge help in debugging this kind of issue. - todd
Re: ksh(1): implement p_tv() with p_ts()
On Tue, 12 Sep 2023 07:49:27 +0200, Theo Buehler wrote: > While this looks like an improvement to me, this uses a new non-portable > construct in ksh. I don't know how much we care. I don't think we care. If someone wants to ports our ksh it is easy enough to supply TIMEVAL_TO_TIMESPEC if necessary. - todd
Re: ksh(1): implement p_tv() with p_ts()
On Mon, 11 Sep 2023 22:10:49 -0500, Scott Cheloha wrote: > p_tv() is identical to p_ts() in every way except for the subsecond > conversion constants. > > Better to write p_ts() once: in p_tv(), convert from timeval to > timespec and call p_ts(). OK millert@ - todd
Re: clang 15 and zlib
On Wed, 06 Sep 2023 10:43:34 +1000, Jonathan Gray wrote: > tb updated us to the newer version a while ago OK millert@ - todd
Re: pax(1) and tar(1): fix misleading -DSMALL
On Mon, 04 Sep 2023 13:54:18 +0100, Jeremie Courreges-Anglas wrote: > Two code sets are currently guarded with #ifdef SMALL in pax(1) and > tar(1): reading 'pax' format extended headers, and identifying various > compressed formats for user-friendliness. As noted by Caspar, the SMALL > path isn't currently used on the install media. I've been confused by > this twice already... > > Here's a proposal: > > 1. always compile in read support for the 'pax' format extended headers. > The ustar format is limited and being able to restore archives using > the pax format in any situation would be nice. Especially if we > switch to writing out pax format archives by default one day. > We're definitely not there yet. Agreed. > 2. actually use -DSMALL to save a bit of storage on the install media. > The behavior is still sane, tar(1) warns that it doesn't recognize > a compressed archive, seeks through it trying to look a tar header, > and eventually gives up. Here's the tiny size change on amd64: > shannon /usr/src/distrib/special/pax$ size tar.o options.o pax obj/tar.o > obj/options.o obj/pax > textdatabss dec hex > 68210 40 68611acdtar.o > 7195108432 83112077options.o > 390495 19024 85392 494911 78d3f pax > 68210 40 68611acdobj/tar.o > 6878108432 79941f3aobj/options.o > 390175 19024 85392 494591 78bff obj/pax > > I don't expect any regression on the ramdisks but a make release is > running just in case. Sure, nothing relies on that anyway. - todd
Re: fw_update lock_db should exit when parent exits
On Wed, 23 Aug 2023 18:23:40 -0700, Andrew Hewus Fresh wrote:
> I would have to see an example of doing that between ksh and perl.
Standard output should already be a pipe in the perl process by
virtue of running as a co-process. In theory you should be able
to poll on it checking for POLLHUP. Since our pipes are actually
bidiretional we can cheat and use select. Something like this:
- todd
lock_db() {
[ "${LOCKPID:-}" ] && return 0
# The installer doesn't have perl, so we can't lock there
[ -e /usr/bin/perl ] || return 0
set -o monitor
perl <<'EOL' |&
use v5.16;
use warnings;
no lib ('/usr/local/libdata/perl5/site_perl');
use OpenBSD::PackageInfo qw< lock_db >;
$|=1;
lock_db(0);
say $$;
my $rin = my $win = my $ein = '';
vec($ein, fileno(STDOUT), 1) = 1;
vec($rin, fileno(STDOUT), 1) = 1;
my $nfound = select(my $rout = $rin, my $wout = $win, my $eout
= $ein, undef);
EOL
set +o monitor
read -rp LOCKPID
return 0
}
Re: fw_update lock_db should exit when parent exits
On Tue, 22 Aug 2023 19:55:56 -0700, Andrew Hewus Fresh wrote: > I noticed this when testing how signal handling worked in fw_update, it > turns out that if you `pkill -KILL -f fw_update` it may leave behind a perl > process that is locking the package database. Instead of just waiting > to be killed, we can have that process check to see if its parent is > still around and exit if not. > > Is there a more appropriate solution to this? > What's the right way to notice your parent exited? One way is to have a pipe between the parent and child and use select() instead of the sleep() to tell when it goes away. - todd
Re: ualarm.3: cleanup, rewrites
On Sun, 30 Jul 2023 20:20:31 -0500, Scott Cheloha wrote: > This patch drags ualarm.3 over to where alarm.3 is. I think it reads > better and the wording is truer to what the function actually does. > In particular, ualarm(3) does not block or "wait": the alarm is > scheduled for asynchronous delivery by the operating system. > > I think I may have tried to clean this up two years ago. I don't > remember where that got sidetracked, but fwiw this was written from > scratch using alarm.3 as a guide. > > Two things I'm iffy on: > > - "high resolution" or "high-resolution"? nanosleep(2) uses "high resolution" so I think we should be consistent with that. > - The current manual mentions an upper bound (2147483647). I'm not > sure when, if ever, this was the real: useconds_t is unsigned, so an > upper bound of INT32_MAX seems off. This may have been copy pasta from alarm.3 which used to document the same limit (which was actually incorrect at the time due to itimerfix). > I am leaning toward just leaving the patch as-is instead of trying > to guide the end-user through the minefield of matching bespoke > "_t" types to real types and limits. > > Tweaks? ok? OK millert@ - todd
Re: cron -n/-s/-q whitespace and /etc/crontab
On Wed, 19 Jul 2023 16:44:23 +0100, Stuart Henderson wrote:
> When /etc/crontab is used, cron only skips over a single whitespace
> character between the username and -n/-s/-q flags; more than one and
> the flag is taken as part of the command:
>
> printf '*\t*\t*\t*\t*\tnobody\t-n true 1\n' | doas tee -a /etc/crontab
> printf '*\t*\t*\t*\t*\tnobody\t\t-n true 2\n' | doas tee -a /etc/crontab
>
> 2023-07-19T15:39:01.316Z symphytum cron[96294]: (nobody) CMD ( -n true 2)
> 2023-07-19T15:39:01.317Z symphytum cron[81012]: (nobody) CMD (true 1)
>
> Is this a "so don't do that then", or is it expected to work?
> (There's no problem with "per-user crontab" files).
It is a bug. There is a missing call to Skip_Blanks() for the
/etc/crontab case. Instead of adding yet another unget_char() after
Skip_Blanks(), we can get rid of a superfluous unget_char() +
get_char() pair.
- todd
Index: usr.sbin/cron/entry.c
===
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.58
diff -u -p -u -r1.58 entry.c
--- usr.sbin/cron/entry.c 13 Jun 2023 15:36:21 - 1.58
+++ usr.sbin/cron/entry.c 19 Jul 2023 16:01:08 -
@@ -275,18 +275,17 @@ load_entry(FILE *file, void (*error_func
goto eof;
}
- /* ch is the first character of a command, or a username */
- unget_char(ch, file);
-
if (!pw) {
char*username = cmd;/* temp buffer */
+ unget_char(ch, file);
ch = get_string(username, MAX_COMMAND, file, " \t\n");
if (ch == EOF || ch == '\n' || ch == '*') {
ecode = e_cmd;
goto eof;
}
+ Skip_Blanks(ch, file)
pw = getpwnam(username);
if (pw == NULL) {
@@ -356,7 +355,6 @@ load_entry(FILE *file, void (*error_func
/* An optional series of '-'-prefixed flags in getopt style can
* occur before the command.
*/
- ch = get_char(file);
while (ch == '-') {
int flags = 0, loop = 1;
Re: ftp: drop needless strcspn
On Wed, 28 Jun 2023 18:37:43 +0200, Omar Polo wrote:
> since fetch.c revision 1.211 ("strip spaces at end of header lines and
> in chunked encoding headers") ftp removes trailing whitespaces early
> so we don't need to re-do that upon every header we parse.
>
> it can also be misleading since one can wonder why LAST_MODIFIED
> doesn't have " \t" but only "\t", or confront it with rpki-client'
> http.c and notice that there there is no strcspn() call in
> Last-Modified handling.
>
> I've tested it by modifying httpd(8) to append " \t " at the end of
> every header (legal per rfc 7230) and observing that ftp still works
> fine, including mtime handling.
OK millert@
- todd
pax: truncate times to MAX_TIME_T, not INT_MAX
If the mtime in the file header is larger than MAX_TIME_T, trucate
it to MAX_TIME_T, not INT_MAX. The existing assignment dates from
before we had a MAX_TIME_T definition in pax.
OK?
- todd
Index: cpio.c
===
RCS file: /cvs/src/bin/pax/cpio.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 cpio.c
--- cpio.c 16 Sep 2017 07:42:34 - 1.33
+++ cpio.c 26 Jun 2023 17:05:35 -
@@ -294,7 +294,7 @@ cpio_rd(ARCHD *arcn, char *buf)
arcn->sb.st_rdev = (dev_t)asc_ul(hd->c_rdev, sizeof(hd->c_rdev), OCT);
val = asc_ull(hd->c_mtime, sizeof(hd->c_mtime), OCT);
if (val > MAX_TIME_T)
- arcn->sb.st_mtime = INT_MAX;/* XXX 2038 */
+ arcn->sb.st_mtime = MAX_TIME_T;
else
arcn->sb.st_mtime = val;
arcn->sb.st_mtim.tv_nsec = 0;
Index: tar.c
===
RCS file: /cvs/src/bin/pax/tar.c,v
retrieving revision 1.70
diff -u -p -u -r1.70 tar.c
--- tar.c 1 Mar 2022 21:19:11 - 1.70
+++ tar.c 26 Jun 2023 17:05:35 -
@@ -411,7 +411,7 @@ tar_rd(ARCHD *arcn, char *buf)
arcn->sb.st_size = (off_t)asc_ull(hd->size, sizeof(hd->size), OCT);
val = asc_ull(hd->mtime, sizeof(hd->mtime), OCT);
if (val > MAX_TIME_T)
- arcn->sb.st_mtime = INT_MAX;/* XXX 2038 */
+ arcn->sb.st_mtime = MAX_TIME_T;
else
arcn->sb.st_mtime = val;
arcn->sb.st_ctime = arcn->sb.st_atime = arcn->sb.st_mtime;
@@ -788,7 +788,7 @@ reset:
if (arcn->sb.st_mtime == 0) {
val = asc_ull(hd->mtime, sizeof(hd->mtime), OCT);
if (val > MAX_TIME_T)
- arcn->sb.st_mtime = INT_MAX;/* XXX 2038 */
+ arcn->sb.st_mtime = MAX_TIME_T;
else
arcn->sb.st_mtime = val;
}
Re: posix_spawn(3): explain that handling NULL envp is an extension
On Mon, 26 Jun 2023 17:24:38 +0200, Paul de Weerd wrote: > Having never heard of posix_spawn(3), I read the full manpage and > (besides wondering "what's the point"), found that it's misspelled Ed > Schouten's name: Yes, that should be fixed. - todd
Re: csh(1), ksh(1), time(1): print durations with millisecond precision
Other implementations of "time -p" (both builtin and standalone) only display two digits after the radix point. I'm a little concerned about breaking scripts that consume out the output of "time -p". Changing the precission of the non-portable output format is fine. - todd
Re: smtpd: allow arguments on NOOP
On Fri, 23 Jun 2023 11:58:47 +0200, Omar Polo wrote: > another diff from the -portable repo: > > https://github.com/OpenSMTPD/OpenSMTPD/pull/1150 > > per rfc-5321 ยง 4.1.1.9 the NOOP command allows optionally one argument > that we SHOULD ignore. > > The original diff set the check function in the commands table to NULL > because NOOP is not a phase that can have filters. However, I prefer > to stay on the safe side and add a smtp_check_noop() function. > Alternatively, we could allow a NULL check function in > smtp_session_imsg(). > > the rfc specifies only one optional string, while here for semplicity > it's relaxed to allow anything. This is a case where being liberal in what you accept seems fine. OK millert@ - todd
Re: Error in ex(1) s command when using c option and numbering on
On Tue, 07 Feb 2023 20:35:10 -0700, Todd C. Miller wrote:
> On Tue, 07 Feb 2023 17:17:02 -0700, Todd C. Miller wrote:
>
> > Yes, the bug is that the number is not displayed. The following
> > diff fixes that but there is still a bug because the resulting line
> > also lacks a line number. In other words, instead of:
> >
> > :s/men/MEN/c
> > 1 Five women came to the party.
> >^^^[ynq]y
> > Five woMEN came to the party.
> >
> > it should look like this:
> >
> > :s/men/MEN/c
> > 1 Five women came to the party.
> >^^^[ynq]y
> > 1 Five woMEN came to the party.
>
> Here's an updated diff that prints line numbers when autoprint is
> set too. This seems to match historic ex behavior and POSIX, but
> I'd appreciate other eyes on it.
Moving this from busg@ to tech@. I noticed today I still have this
diff from Feb rotting in my tree. The original thread is:
https://marc.info/?l=openbsd-bugs=167580085421828=2
OK?
Index: usr.bin/vi/ex/ex.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 ex.c
--- usr.bin/vi/ex/ex.c 20 Feb 2022 19:45:51 - 1.22
+++ usr.bin/vi/ex/ex.c 8 Feb 2023 03:28:32 -
@@ -1454,8 +1454,14 @@ addr_verify:
LF_INIT(FL_ISSET(ecp->iflags, E_C_HASH | E_C_LIST | E_C_PRINT));
if (!LF_ISSET(E_C_HASH | E_C_LIST | E_C_PRINT | E_NOAUTO) &&
!F_ISSET(sp, SC_EX_GLOBAL) &&
- O_ISSET(sp, O_AUTOPRINT) && F_ISSET(ecp, E_AUTOPRINT))
- LF_INIT(E_C_PRINT);
+ O_ISSET(sp, O_AUTOPRINT) && F_ISSET(ecp, E_AUTOPRINT)) {
+
+ /* Honor the number option if autoprint is set. */
+ if (F_ISSET(ecp, E_OPTNUM))
+ LF_INIT(E_C_HASH);
+ else
+ LF_INIT(E_C_PRINT);
+ }
if (LF_ISSET(E_C_HASH | E_C_LIST | E_C_PRINT)) {
cur.lno = sp->lno;
Index: usr.bin/vi/ex/ex_subst.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_subst.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ex_subst.c
--- usr.bin/vi/ex/ex_subst.c18 Apr 2017 01:45:35 - 1.30
+++ usr.bin/vi/ex/ex_subst.c8 Feb 2023 03:23:27 -
@@ -633,7 +633,9 @@ nextmatch: match[0].rm_so = offset;
goto lquit;
}
} else {
- if (ex_print(sp, cmdp, , , 0) ||
+ const int flags =
+ O_ISSET(sp, O_NUMBER) ? E_C_HASH : 0;
+ if (ex_print(sp, cmdp, , , flags) ||
ex_scprint(sp, , ))
goto lquit;
if (ex_txt(sp, , 0, TXT_CR))
Re: avoid truncation of filtered smtpd data lines
On Wed, 21 Jun 2023 19:11:09 +0200, Omar Polo wrote:
> On 2023/06/20 14:38:37 -0600, Todd C. Miller wrote:
> > > qid = ep+1;
> > > - if ((ep = strchr(qid, '|')) == NULL)
> > > - fatalx("Missing reqid: %s", line);
> > > - ep[0] = '\0';
> > > -
> >
> > This is not a new problem but we really should set errno=0 before
> > calling strtoull() for the first time. It is possible for errno
> > to already be set to ERANGE which causes problems if strtoull()
> > returns ULLONG_MAX and there is not an error. It's fine if you
> > don't want to fix that in this commit but I do think it should get
> > fixed.
>
> if you don't mind i'll fix it in a separate commit. I've also checked
> if there were other places to adjust but this appears to be the only
> one instance.
That's perfectly fine.
> > It took me a minute to realize that this is OK, but it seems fine.
> >
> > > if (strcmp(response, "proceed") == 0) {
> > > - if (parameter != NULL)
> > > - fatalx("Unexpected parameter after proceed: %s", line);
> > > filter_protocol_next(token, reqid, 0);
> > > return;
>
> yeah it's subtle, i should have pointed it out, sorry. if "response"
> contain a parameter, strcmp() return nonzero, so the parameter check
> is not needed.
>
> The drawback is that the error messages on protocol violation are a
> bit worst. Before something like "...|proceed|foobar" would fail with
> "unexpected parameter after proceed: ", now "Invalid directive:
> ", but I don't think it's a big deal.
I noticed this and I agree that it is not a big deal. If you are
writing a filter you should know enough to debug the problem with
the amount of detail provided.
OK millert@ for the diff as-is if it was not obvious from my previous
reply.
- todd
Re: avoid truncation of filtered smtpd data lines
On Tue, 20 Jun 2023 21:38:49 +0200, Omar Polo wrote:
> Then I realized that we don't need to copy the line at all. We're
> already using strtoull to parse the number and the payload is the last
> field of the received line, so we can just advance the pointer. The
> drawback is that we now need to use a few strncmp, but I think it's
> worth it.
This seems like a good approach, minor comments inline.
- todd
> diff /usr/src
> commit - 5c586f5f5360442b12bbc4ea18ce006ea0c3d126
> path + /usr/src
> blob - a714446c26fee299f4450ff1ad40289b5b327824
> file + usr.sbin/smtpd/lka_filter.c
> --- usr.sbin/smtpd/lka_filter.c
> +++ usr.sbin/smtpd/lka_filter.c
> @@ -593,40 +593,27 @@ lka_filter_process_response(const char *name, const ch
> {
> uint64_t reqid;
> uint64_t token;
> - char buffer[LINE_MAX];
> char *ep = NULL;
> - char *kind = NULL;
> - char *qid = NULL;
> - /*char *phase = NULL;*/
> - char *response = NULL;
> - char *parameter = NULL;
> + const char *kind = NULL;
> + const char *qid = NULL;
> + const char *response = NULL;
> + const char *parameter = NULL;
> struct filter_session *fs;
>
> - (void)strlcpy(buffer, line, sizeof buffer);
> - if ((ep = strchr(buffer, '|')) == NULL)
> + kind = line;
> +
> + if ((ep = strchr(kind, '|')) == NULL)
> fatalx("Missing token: %s", line);
> - ep[0] = '\0';
> -
> - kind = buffer;
> -
> qid = ep+1;
> - if ((ep = strchr(qid, '|')) == NULL)
> - fatalx("Missing reqid: %s", line);
> - ep[0] = '\0';
> -
This is not a new problem but we really should set errno=0 before
calling strtoull() for the first time. It is possible for errno
to already be set to ERANGE which causes problems if strtoull()
returns ULLONG_MAX and there is not an error. It's fine if you
don't want to fix that in this commit but I do think it should get
fixed.
> reqid = strtoull(qid, , 16);
> - if (qid[0] == '\0' || *ep != '\0')
> + if (qid[0] == '\0' || *ep != '|')
> fatalx("Invalid reqid: %s", line);
> if (errno == ERANGE && reqid == ULLONG_MAX)
> fatal("Invalid reqid: %s", line);
>
> - qid = ep+1;
> - if ((ep = strchr(qid, '|')) == NULL)
> - fatal("Missing directive: %s", line);
> - ep[0] = '\0';
> -
> + qid = ep + 1;
> token = strtoull(qid, , 16);
> - if (qid[0] == '\0' || *ep != '\0')
> + if (qid[0] == '\0' || *ep != '|')
> fatalx("Invalid token: %s", line);
> if (errno == ERANGE && token == ULLONG_MAX)
> fatal("Invalid token: %s", line);
> @@ -637,7 +624,7 @@ lka_filter_process_response(const char *name, const ch
> if ((fs = tree_get(, reqid)) == NULL)
> return;
>
> - if (strcmp(kind, "filter-dataline") == 0) {
> + if (strncmp(kind, "filter-dataline|", 16) == 0) {
> if (fs->phase != FILTER_DATA_LINE)
> fatalx("filter-dataline out of dataline phase");
> filter_data_next(token, reqid, response);
> @@ -646,19 +633,13 @@ lka_filter_process_response(const char *name, const ch
> if (fs->phase == FILTER_DATA_LINE)
> fatalx("filter-result in dataline phase");
>
> - if ((ep = strchr(response, '|'))) {
> + if ((ep = strchr(response, '|')) != NULL)
> parameter = ep + 1;
> - ep[0] = '\0';
> - }
>
It took me a minute to realize that this is OK, but it seems fine.
> if (strcmp(response, "proceed") == 0) {
> - if (parameter != NULL)
> - fatalx("Unexpected parameter after proceed: %s", line);
> filter_protocol_next(token, reqid, 0);
> return;
> } else if (strcmp(response, "junk") == 0) {
> - if (parameter != NULL)
> - fatalx("Unexpected parameter after junk: %s", line);
> if (fs->phase == FILTER_COMMIT)
> fatalx("filter-reponse junk after DATA");
> filter_result_junk(reqid);
> @@ -667,11 +648,11 @@ lka_filter_process_response(const char *name, const ch
> if (parameter == NULL)
> fatalx("Missing parameter: %s", line);
>
> - if (strcmp(response, "rewrite") == 0)
> + if (strncmp(response, "rewrite|", 8) == 0)
> filter_result_rewrite(reqid, parameter);
> - else if (strcmp(response, "reject") == 0)
> + else if (strncmp(response, "reject|", 7) == 0)
> filter_result_reject(reqid, parameter);
> - else if (strcmp(response, "disconnect") == 0)
> + else if (strncmp(response, "disconnect|", 11) == 0)
> filter_result_disconnect(reqid, parameter);
> else
> fatalx("Invalid directive: %s", line);
>
Re: open_memstream cleanup
On Tue, 20 Jun 2023 17:49:46 +0200, Claudio Jeker wrote: > In open_memstream() the code does a bzero() of the new memory even though > recallocarray() used which does this already. > > In open_wmemstream() the code does the same but is still using > reallocarray(). So adjust that code to be like open_memstream(). OK millert@ - todd
Re: smtpd: sync imsg_to_str()
On Sun, 18 Jun 2023 16:49:30 +0200, Omar Polo wrote: > some imsg types are missing from the big switch in imsg_to_str(), > noticed after a report in m...@opensmtpd.org. Tracing shows: > > : imsg: lka <- dispatcher: IMSG_??? (139) (len=42) > > (imsg #139 should be IMSG_REPORT_SMTP_FILTER_RESPONSE if I'm counting > right.) > > Instead of checking one by one (they're a lot!) I just copied over the > list from smtpd.h and ran an emacs macro. Some entries changed place, > but since the list is long I figured this was the best way to keep > everything in sync. Using the same order as smtpd.h makes sense. OK millert@ - todd
Re: smtpd-filters: swap link-auth fields
On Wed, 14 Jun 2023 16:34:39 +0200, Omar Polo wrote: > the `link-auth' event hash the user first and the result of the > operation after; this breaks when a username has a '|' character in > it. Since this is triggered by the `auth login' command, anyone could > send a user with a '|' and, depending on the filter used, make smtpd > exit. (if the filter dies, smtpd does too) > > This was reported on the OpenSMTPD-portable github repository with > Gilles' opensmtpd-filter-rspamd: > > https://github.com/OpenSMTPD/OpenSMTPD/issues/1213 > > Diff below is straightforward and includes the documentation changes. > I believe link-auth was forgotten in revision 1.61 of lka_filter.c > when the mail-from/rcpt-to events got their fields swapped. OK millert@ - todd
Re: seq: fix check for rounding error/truncation
On Mon, 12 Jun 2023 18:48:56 +0100, Stuart Henderson wrote: > Neither of these are really ideal, and mean that we still need gseq > for integers >= 1,000,000 > > $ seq 99 99 > 99 > $ seq 100 100 > 1e+06 > > $ gseq 105 105 > 105 That probably requires a separate code path specifically for integers. One thing at a time. - todd
Re: seq: fix check for rounding error/truncation
For context, see: https://chaos.social/@Gottox/110527807405964874 https://github.com/chimera-linux/chimerautils/commit/1ecc1e99d4a309631e846a868b5a422f996704ac
seq: fix check for rounding error/truncation
We need to compare the printable version of the last value displayed,
not the floating point representation. Otherwise, we may print the
last value twice.
Old:
$ seq 105 105
1.05e+06
1.05e+06
New:
$ seq 105 105
1.05e+06
We really need seq regression tests. I have a few that I will
commit after this is in.
- todd
Index: usr.bin/seq/seq.c
===
RCS file: /cvs/src/usr.bin/seq/seq.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 seq.c
--- usr.bin/seq/seq.c 25 Feb 2022 16:00:39 - 1.6
+++ usr.bin/seq/seq.c 12 Jun 2023 17:13:44 -
@@ -89,13 +89,13 @@ main(int argc, char *argv[])
double first = 1.0;
double last = 0.0;
double incr = 0.0;
- double last_shown_value = 0.0;
+ double prev = 0.0;
double cur, step;
struct lconv *locale;
char *fmt = NULL;
const char *sep = "\n";
const char *term = "\n";
- char *cur_print, *last_print;
+ char *cur_print, *last_print, *prev_print;
char pad = ZERO;
if (pledge("stdio", NULL) == -1)
@@ -181,29 +181,31 @@ main(int argc, char *argv[])
if (cur != first)
fputs(sep, stdout);
printf(fmt, cur);
- last_shown_value = cur;
+ prev = cur;
}
/*
* Did we miss the last value of the range in the loop above?
*
* We might have, so check if the printable version of the last
-* computed value ('cur') and desired 'last' value are equal. If they
-* are equal after formatting truncation, but 'cur' and
-* 'last_shown_value' are not equal, it means the exit condition of the
-* loop held true due to a rounding error and we still need to print
-* 'last'.
+* computed value ('cur') and desired 'last' value are equal. If
+* they are equal after formatting truncation, but 'cur' and 'prev'
+* are different, it means the exit condition of the loop held true
+* due to a rounding error and we still need to print 'last'.
*/
if (asprintf(_print, fmt, cur) == -1 ||
- asprintf(_print, fmt, last) == -1)
+ asprintf(_print, fmt, last) == -1 ||
+ asprintf(_print, fmt, prev) == -1)
err(1, "asprintf");
- if (strcmp(cur_print, last_print) == 0 && cur != last_shown_value) {
+ if (strcmp(cur_print, last_print) == 0 &&
+ strcmp(cur_print, prev_print) != 0) {
if (cur != first)
fputs(sep, stdout);
fputs(last_print, stdout);
}
free(cur_print);
free(last_print);
+ free(prev_print);
fputs(term, stdout);
Re: acme-client(8): preliminary support for HiCA
On Fri, 09 Jun 2023 07:25:04 +0200, Florian Obser wrote: > OK? > > p.s. I'm currently busy writing an ISC licensed bash in rust to safely > support HiCA. So this might take a while... Have you considered implementing wordexp(3) to allow command substitution? It may be necessary to add inline support for IFS to fully support your use case. Also, for full compatibility I think it would be better to choose a User-Agent similar to: Mozilla/5.0 (OpenBSD 7.3; acme-client; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.5112.34 Safari/537.36 obviously you need to substitute in the OpenBSD version and architecture. - todd
Re: switch mail.local to getline()
On Sat, 03 Jun 2023 11:55:46 +0200, Omar Polo wrote: > As per subject. While here I couldn't resist simplifying the "From " > check too, although it is indipendent from the rest of the diff. > (could commit separately if preferred.) OK millert@ - todd
Re: smtpd: add missing time.h include
On Wed, 31 May 2023 11:00:37 +0200, Omar Polo wrote: > After a report of a build fail with some old gcc on RHEL7 / Centos, I > noticed that we're lacking the include time.h for time(3), > clock_gettime(3) and localtime(3). Diff below adds it in all the > missing files. I'm also including sys/time.h in smtpd.h, as noted in > event_init(3), since we're including event.h. OK millert@ - todd
Re: ksh, test: test -t file descriptor isn't optional
I just checked and it seems that while zsh supports the pre-POSIX behavior of "test -t" using fd 1, both dash and bash treat "-t" as a string by default when there is no fd specified. So I think this change is fine. OK millert@ for your revised dif if you add the missing ';' after the "return 0". - todd
Re: Bail out on "id -R user"
On Tue, 30 May 2023 09:47:08 +0200, Omar Polo wrote: > fwiw I agree. Although it doesn't makes much sense to pass an > argument to id -R, not failing may give the wrong impression. > > ok op@ (i'll wait a bit before commiting this in case someone > disagrees) OK millert@ as well if you want to commit it. - todd
Re: userdel: remove login group for =uid
On Thu, 25 May 2023 06:54:08 +0100, Stuart Henderson wrote: > > As Aisha pointed out, pkg_delete hints could be updated too. > > If that is done, pkg_delete would need to check whether the group will > actually get removed i.e. make sure that no other user has been added > to the group. If pkg_delete is going to rely on userdel removing the group I think I need to remove the "=uid" check in my userdel patch. In other words, the primary group will always be removed if it matches the username, the uid and gid are the same and the group has no other members. That is also a lot easier to document. - todd
Re: Installer: use $(
On Tue, 23 May 2023 23:41:32 +0200, Christian Weisgerber wrote:
> This replaces "$(cat file)" with the ksh construct "$( Admittedly cosmetic.
>
> I have left the line
>
> local _sec=$(cat $HTTP_SEC 2>/dev/null)
>
> unchanged, since it would require
>
> { local var=$(<$HTTP_SEC); } 2>/dev/null
>
> which is sufficiently opaque that I'm not sure it's an improvement.
OK millert@
- todd
Re: sticky(8): mark S_ISVTX as Dv
On Wed, 24 May 2023 16:04:13 +0200, Omar Polo wrote: > It makes `man -k any=S_ISVTX' slightly more useful by pointing at > sticky(8) too other than strmode(3); may help if someone (like me :-) > forgot about sticky(8) files. OK millert@ - todd
Re: Installer: use $(
On Tue, 23 May 2023 22:22:04 -, Klemens Nanni wrote:
> I'm pointing this out because the error message we'd get provides less
> information with your diff:
>
> $ echo $(cat /nope) 2>/dev/null
> cat: /nope: No such file or directory
> vs.
> echo $(< /nope) 2>/dev/null
> ksh: /nope: cannot open $(<) input
>
> But given the constraint environment, I'm fine with turning those four
> $(cat ...) instances into $(< ...) like the exising seven onces with
> the intial example being the only outlier.
Here's the error output from bash, zsh and ksh93:
$bash echo $(< /nope)
bash: /nope: No such file or directory
$zsh echo $(< /nope)
zsh: no such file or directory: /nope
$ksh93 echo $(< /nope)
ksh93: /nope: cannot open [No such file or directory]
Maybe we should just adjust ksh to display the more useful message?
With diff:
$ echo $(< /nope)
ksh: /nope: No such file or directory
- todd
diff -u -p -u -r1.66 eval.c
--- bin/ksh/eval.c 13 Sep 2020 15:39:09 - 1.66
+++ bin/ksh/eval.c 24 May 2023 14:03:41 -
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
#include
@@ -909,7 +910,7 @@ comsub(Expand *xp, char *cp)
SHF_MAPHI|SHF_CLEXEC);
if (shf == NULL)
warningf(!Flag(FTALKING),
- "%s: cannot open $(<) input", name);
+ "%s: %s", name, strerror(errno));
xp->split = 0; /* no waitlast() */
} else {
int ofd1, pv[2];
Re: userdel: remove login group for =uid
On Fri, 19 May 2023 12:52:30 -0400, A Tammy wrote: > Will this not potentially break people's setups? I like this change though. I hope not. The group would only be removed if it matches the user name and ID and there are no other members. > This would also make the post pkg_delete output simpler by just asking > the admin to delete the user, instead of both user and group. Right. - todd
userdel: remove login group for =uid
If /etc/usermgmt.conf has a line like:
group =uid
where a new user's group ID in the passwd file is the same as their
user ID, remove that group when the user is removed. The group is
only removed if it matches the login name, has a gid that matches
the user's uid, and has no other members.
This makes our userdel(8) behave more like the version on other
systems.
Opinions? This is something that has always bothered me and can
result in uid/gid mismatches if you remove a user, then re-add them
without removing the login group first.
Thoughts or strong opinions?
- todd
Index: usr.sbin/user/user.c
===
RCS file: /cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.131
diff -u -p -u -r1.131 user.c
--- usr.sbin/user/user.c18 May 2023 18:29:28 - 1.131
+++ usr.sbin/user/user.c19 May 2023 16:16:02 -
@@ -193,7 +193,7 @@ static int is_local(char *, const char *
static int modify_gid(char *, char *);
static int moduser(char *, char *, user_t *);
static int removehomedir(const char *, uid_t, const char *);
-static int rm_user_from_groups(char *);
+static int rm_user_from_groups(char *, int);
static int save_range(user_t *, char *);
static int scantime(time_t *, char *);
static int setdefaults(user_t *);
@@ -1308,9 +1308,9 @@ adduser(char *login_name, user_t *up)
return 1;
}
-/* remove a user from the groups file */
+/* remove a user from the groups file, optionally removing the login group */
static int
-rm_user_from_groups(char *login_name)
+rm_user_from_groups(char *login_name, int rm_login_group)
{
struct stat st;
size_t login_len;
@@ -1366,6 +1366,15 @@ rm_user_from_groups(char *login_name)
warnx("Malformed entry `%s'. Skipping", buf);
continue;
}
+ if (rm_login_group && strncmp(buf, login_name, login_len) == 0
+ && buf[login_len] == ':') {
+ /* remove login group if empty or user is only member */
+ if (*cp == '\n')
+ continue;
+ if (strncmp(cp, login_name, login_len) == 0 &&
+ cp [login_len] == '\n')
+ continue;
+ }
while ((cp = strstr(cp, login_name)) != NULL) {
if ((cp[-1] == ':' || cp[-1] == ',') &&
(cp[login_len] == ',' || cp[login_len] == '\n')) {
@@ -1745,7 +1754,7 @@ moduser(char *login_name, char *newlogin
up->u_groupv[i]);
}
}
- if (!rm_user_from_groups(newlogin)) {
+ if (!rm_user_from_groups(newlogin, 0)) {
close(ptmpfd);
pw_abort();
errx(EXIT_FAILURE,
@@ -2101,8 +2110,10 @@ int
userdel(int argc, char **argv)
{
struct passwd *pwp;
+ struct group*grp;
user_t u;
int defaultfield;
+ int rm_login_group;
int rmhome;
int bigD;
int c;
@@ -2164,7 +2175,15 @@ userdel(int argc, char **argv)
openlog("userdel", LOG_PID, LOG_USER);
return moduser(*argv, *argv, ) ? EXIT_SUCCESS : EXIT_FAILURE;
}
- if (!rm_user_from_groups(*argv)) {
+ rm_login_group = 0;
+ if (strcmp(u.u_primgrp, "=uid") == 0 && pwp->pw_uid == pwp->pw_gid) {
+ /* remove primary group if it matches the username */
+ grp = getgrgid(pwp->pw_gid);
+ if (grp != NULL && strcmp(grp->gr_name, *argv) == 0) {
+ rm_login_group = 1;
+ }
+ }
+ if (!rm_user_from_groups(*argv, rm_login_group)) {
return 0;
}
openlog("userdel", LOG_PID, LOG_USER);
Re: smtpd.conf.5: fix markup for action maildir
On Fri, 19 May 2023 13:06:32 +0200, Omar Polo wrote: > it's currently rendered as > > maildir [pathname [junk]] > > whereas it really is > > maildir [pathname] [junk] > > i.e. both the path and `junk' are optional but indipendently so. it's > quite clear from the grammar in parse.y. OK millert@ - todd
Re: user: handle paths with whitespace / metacharacters
On Thu, 18 May 2023 18:13:57 +0200, Omar Polo wrote:
> existing code just used 0 and 1 (e.g. creategid.) lone zeros as
> arguments are not really readable, but still I can't make myself
> liking stdbool. not that my tastes matters, but it seems to be used
> very sparsingly in usr.sbin
This was leftover from before I added bit flags to runas().
It is not actually needed now so I have removed it.
> > #include
> > #include
> > #include
> > @@ -103,6 +104,10 @@ enum {
> > F_ACCTUNLOCK= 0x1
> > };
> >
> > +/* bit flags for runas() */
> > +#define RUNAS_DUP_DEVNULL 0x01
> > +#define RUNAS_IGNORE_EXITVAL 0x02
>
> another nit: the other flags are defined within an enums.
Sure, no reason to be inconsistent.
> > [...]
> > -/* a replacement for system(3) */
> > +/* run the given command with optional arguments as the specified uid */
> > static int
> > -asystem(const char *fmt, ...)
> > +runas(const char *path, const char *const argv[], uid_t uid, int flags)
> > {
> > [...]
> > + default:
> > + while (waitpid(child, , 0) == -1) {
> > + if (errno != EINTR)
> > + err(EXIT_FAILURE, "waitpid");
> > + }
> > + if (WIFSIGNALED(status)) {
> > + ret = WTERMSIG(status);
> > + warnx("[Warning] `%s' killed by signal %d", buf, ret);
> > + ret |= 128;
> > + } else {
> > + if (!(flags & RUNAS_IGNORE_EXITVAL))
> > + ret = WEXITSTATUS(status);
> > + if (ret != 0)
> > + warnx("[Warning] unable to run `%s'", buf);
>
> more than "unable to run" I'd say "failed to run" or "command
> failed" / "exited with nonzero status".
How about "failed with status N"? It can be useful to have the
exit status since that may indicate the source of the errorย (though
perhaps not so much in this case).
Updated diff follows.
- todd
Index: usr.sbin/user/user.c
===
RCS file: /cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.130
diff -u -p -u -r1.130 user.c
--- usr.sbin/user/user.c16 May 2023 21:28:46 - 1.130
+++ usr.sbin/user/user.c18 May 2023 17:24:01 -
@@ -31,6 +31,7 @@
#include
#include
+#include
#include
#include
@@ -42,7 +43,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -103,6 +103,12 @@ enum {
F_ACCTUNLOCK= 0x1
};
+/* flags for runas() */
+enum {
+ RUNAS_DUP_DEVNULL = 0x01,
+ RUNAS_IGNORE_EXITVAL= 0x02
+};
+
#define CONFFILE "/etc/usermgmt.conf"
#define _PATH_NONEXISTENT "/nonexistent"
@@ -179,8 +185,6 @@ enum {
static int adduser(char *, user_t *);
static int append_group(char *, int, const char **);
-static int asystem(const char *, ...)
- __attribute__((__format__(__printf__, 1, 2)));
static int copydotfiles(char *, char *);
static int creategid(char *, gid_t, const char *);
static int getnextgid(uid_t *, uid_t, uid_t);
@@ -214,30 +218,79 @@ strsave(char **cpp, const char *s)
err(1, NULL);
}
-/* a replacement for system(3) */
+/* run the given command with optional arguments as the specified uid */
static int
-asystem(const char *fmt, ...)
+runas(const char *path, const char *const argv[], uid_t uid, int flags)
{
- va_list vp;
- charbuf[MaxCommandLen];
- int ret;
+ int argc, status, ret = 0;
+ char buf[MaxCommandLen];
+ pid_t child;
- va_start(vp, fmt);
- (void) vsnprintf(buf, sizeof(buf), fmt, vp);
- va_end(vp);
- if (verbose) {
- printf("Command: %s\n", buf);
+ strlcpy(buf, path, sizeof(buf));
+ for (argc = 1; argv[argc] != NULL; argc++) {
+ strlcat(buf, " ", sizeof(buf));
+ strlcat(buf, argv[argc], sizeof(buf));
}
- if ((ret = system(buf)) != 0) {
- warnx("[Warning] can't system `%s'", buf);
+ if (verbose)
+ printf("Command: %s\n", buf);
+
+ child = fork();
+ switch (child) {
+ case -1:
+ err(EXIT_FAILURE, "fork");
+ case 0:
+ if (flags & RUNAS_DUP_DEVNULL) {
+ /* Redirect output to /dev/null if possible. */
+ int dev_null = open(_PATH_DEVNULL, O_RDWR);
+ if (dev_null != -1) {
+ dup2(dev_null, STDOUT_FILENO);
+ dup2(dev_null, STDERR_FILENO);
+ if (dev_null > STDERR_FILENO)
+ close(dev_null);
+ } else {
+ warn("%s", _PATH_DEVNULL);
+ }
+ }
+ if (uid != -1) {
+ if (setresuid(uid, uid, uid) == -1)
+
user: handle paths with whitespace / metacharacters
The way user(8) runs commands via system(3) is fragile. It does
not correctly handle paths with whitespace or shell metacharacters.
Rather than try to quote everything (which is also fragile) I think
it is safest to just exec the commands directly.
OK?
- todd
Index: usr.sbin/user/user.c
===
RCS file: /cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.130
diff -u -p -u -r1.130 user.c
--- usr.sbin/user/user.c16 May 2023 21:28:46 - 1.130
+++ usr.sbin/user/user.c18 May 2023 15:10:52 -
@@ -31,6 +31,7 @@
#include
#include
+#include
#include
#include
@@ -42,7 +43,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -103,6 +104,10 @@ enum {
F_ACCTUNLOCK= 0x1
};
+/* bit flags for runas() */
+#define RUNAS_DUP_DEVNULL 0x01
+#define RUNAS_IGNORE_EXITVAL 0x02
+
#define CONFFILE "/etc/usermgmt.conf"
#define _PATH_NONEXISTENT "/nonexistent"
@@ -179,8 +184,6 @@ enum {
static int adduser(char *, user_t *);
static int append_group(char *, int, const char **);
-static int asystem(const char *, ...)
- __attribute__((__format__(__printf__, 1, 2)));
static int copydotfiles(char *, char *);
static int creategid(char *, gid_t, const char *);
static int getnextgid(uid_t *, uid_t, uid_t);
@@ -214,30 +217,77 @@ strsave(char **cpp, const char *s)
err(1, NULL);
}
-/* a replacement for system(3) */
+/* run the given command with optional arguments as the specified uid */
static int
-asystem(const char *fmt, ...)
+runas(const char *path, const char *const argv[], uid_t uid, int flags)
{
- va_list vp;
- charbuf[MaxCommandLen];
- int ret;
+ int argc, status, ret = 0;
+ char buf[MaxCommandLen];
+ pid_t child;
- va_start(vp, fmt);
- (void) vsnprintf(buf, sizeof(buf), fmt, vp);
- va_end(vp);
- if (verbose) {
- printf("Command: %s\n", buf);
+ strlcpy(buf, path, sizeof(buf));
+ for (argc = 1; argv[argc] != NULL; argc++) {
+ strlcat(buf, " ", sizeof(buf));
+ strlcat(buf, argv[argc], sizeof(buf));
}
- if ((ret = system(buf)) != 0) {
- warnx("[Warning] can't system `%s'", buf);
+ if (verbose)
+ printf("Command: %s\n", buf);
+
+ child = fork();
+ switch (child) {
+ case -1:
+ err(EXIT_FAILURE, "fork");
+ case 0:
+ if (flags & RUNAS_DUP_DEVNULL) {
+ /* Redirect output to /dev/null if possible. */
+ int dev_null = open(_PATH_DEVNULL, O_RDWR);
+ if (dev_null != -1) {
+ dup2(dev_null, STDOUT_FILENO);
+ dup2(dev_null, STDERR_FILENO);
+ if (dev_null > STDERR_FILENO)
+ close(dev_null);
+ } else {
+ warn("%s", _PATH_DEVNULL);
+ }
+ }
+ if (uid != -1) {
+ if (setresuid(uid, uid, uid) == -1)
+ warn("setresuid(%u, %u, %u)", uid, uid, uid);
+ }
+ execv(path, (char **)argv);
+ warn("%s", buf);
+ _exit(EXIT_FAILURE);
+ default:
+ while (waitpid(child, , 0) == -1) {
+ if (errno != EINTR)
+ err(EXIT_FAILURE, "waitpid");
+ }
+ if (WIFSIGNALED(status)) {
+ ret = WTERMSIG(status);
+ warnx("[Warning] `%s' killed by signal %d", buf, ret);
+ ret |= 128;
+ } else {
+ if (!(flags & RUNAS_IGNORE_EXITVAL))
+ ret = WEXITSTATUS(status);
+ if (ret != 0)
+ warnx("[Warning] unable to run `%s'", buf);
+ }
+ return ret;
}
- return ret;
+}
+
+/* run the given command with optional arguments */
+static int
+run(const char *path, const char *const argv[])
+{
+ return runas(path, argv, -1, false);
}
/* remove a users home directory, returning 1 for success (ie, no problems
encountered) */
static int
removehomedir(const char *user, uid_t uid, const char *dir)
{
+ const char *rm_argv[] = { "rm", "-rf", dir, NULL };
struct stat st;
/* userid not root? */
@@ -263,11 +313,9 @@ removehomedir(const char *user, uid_t ui
return 0;
}
- (void) seteuid(uid);
- /* we add the "|| true" to keep asystem() quiet if there is a non-zero
exit status. */
- (void) asystem("%s -rf %s > /dev/null 2>&1 || true", RM, dir);
- (void) seteuid(0);
-
Re: ix hardware tso
On Tue, 16 May 2023 19:26:07 +0200, Alexander Bluhm wrote: > On Tue, May 16, 2023 at 11:15:31AM -0600, Todd C. Miller wrote: > > Would it be possible to move the forward declaration of struct tdb > > to netinet/tcp_var.h so it is not required in every driver? > > sure Thanks, that looks better to me. - todd
useradd: use "cp" instead of "pax" to copy dot files
We can just use "cp -a skeldir/. homedir" to copy the skeleton dot
files to the new user's homedir. There's no good reason to use pax
when cp will do and this will simplify a future commit of mine.
- todd
Index: usr.sbin/user/user.c
===
RCS file: /cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.129
diff -u -p -u -r1.129 user.c
--- usr.sbin/user/user.c15 May 2023 17:00:24 - 1.129
+++ usr.sbin/user/user.c16 May 2023 17:30:18 -
@@ -171,7 +171,7 @@ enum {
#define MKDIR "/bin/mkdir"
#define MV "/bin/mv"
#define NOLOGIN"/sbin/nologin"
-#define PAX"/bin/pax"
+#define CP "/bin/cp"
#define RM "/bin/rm"
#define UNSET_INACTIVE "Null (unset)"
@@ -311,8 +311,8 @@ copydotfiles(char *skeldir, char *dir)
if (n == 0) {
warnx("No \"dot\" initialisation files found");
} else {
- (void) asystem("cd %s && %s -rw -pe %s . %s",
- skeldir, PAX, (verbose) ? "-v" : "", dir);
+ (void) asystem("%s -a %s %s/. %s",
+ CP, (verbose) ? "-v" : "", skeldir, dir);
}
return n;
}
Re: smtpd: some fatal -> fatalx
On Tue, 16 May 2023 14:51:44 +0200, Omar Polo wrote: > while debugging a pebkac in -portable, I noticed that in various > places we use fatal() for libtls failures. errno doesn't generally > contains anything useful after libtls functions, and in most it's > explicitly cleared to avoid misuse. > > just to provide a quick example, with `listen on ... ciphers foobar': > > % doas smtpd -d > info: OpenSMTPD 7.0.0 starting > dispatcher: no ciphers for 'foobar': No such file or directory > smtpd: process dispatcher socket closed > > So change most of them to fatalx which doesn't append errno. While > here I'm also logging the actual error, via tls_config_error() or > tls_error(), that before was missing. > > tls_config_new(), tls_server() and tls_client() failures are still > logged with fatal(), which I believe it's correct. OK millert@ - todd
user: simplify memsave() to strsave()
All the callers of memsave() pass strlen(s) as the size argument.
We can eliminate the size argument and just use strdup(3) instead.
OK?
- todd
Index: user.c
===
RCS file: /cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.128
diff -u -p -u -r1.128 user.c
--- user.c 17 Oct 2019 21:54:29 - 1.128
+++ user.c 15 May 2023 16:05:40 -
@@ -200,20 +200,18 @@ static size_t expand_len(const char *, c
static struct group *find_group_info(const char *);
static struct passwd *find_user_info(const char *);
static void checkeuid(void);
-static void memsave(char **, const char *, size_t);
+static void strsave(char **, const char *);
static void read_defaults(user_t *);
static int verbose;
-/* if *cpp is non-null, free it, then assign `n' chars of `s' to it */
+/* free *cpp, then strdup `s' to it */
static void
-memsave(char **cpp, const char *s, size_t n)
+strsave(char **cpp, const char *s)
{
free(*cpp);
- if ((*cpp = calloc (n + 1, sizeof(char))) == NULL)
+ if ((*cpp = strdup(s)) == NULL)
err(1, NULL);
- memcpy(*cpp, s, n);
- (*cpp)[n] = '\0';
}
/* a replacement for system(3) */
@@ -788,12 +786,12 @@ read_defaults(user_t *up)
unsigned char *cp;
unsigned char *s;
- memsave(>u_primgrp, DEF_GROUP, strlen(DEF_GROUP));
- memsave(>u_basedir, DEF_BASEDIR, strlen(DEF_BASEDIR));
- memsave(>u_skeldir, DEF_SKELDIR, strlen(DEF_SKELDIR));
- memsave(>u_shell, DEF_SHELL, strlen(DEF_SHELL));
- memsave(>u_comment, DEF_COMMENT, strlen(DEF_COMMENT));
- memsave(>u_class, DEF_CLASS, strlen(DEF_CLASS));
+ strsave(>u_primgrp, DEF_GROUP);
+ strsave(>u_basedir, DEF_BASEDIR);
+ strsave(>u_skeldir, DEF_SKELDIR);
+ strsave(>u_shell, DEF_SHELL);
+ strsave(>u_comment, DEF_COMMENT);
+ strsave(>u_class, DEF_CLASS);
up->u_rsize = 16;
up->u_defrc = 0;
if ((up->u_rv = calloc(up->u_rsize, sizeof(range_t))) == NULL)
@@ -811,27 +809,27 @@ read_defaults(user_t *up)
if (strncmp(s, "group", 5) == 0) {
for (cp = s + 5 ; isspace((unsigned char)*cp);
cp++) {
}
- memsave(>u_primgrp, cp, strlen(cp));
+ strsave(>u_primgrp, cp);
} else if (strncmp(s, "base_dir", 8) == 0) {
for (cp = s + 8 ; isspace((unsigned char)*cp);
cp++) {
}
- memsave(>u_basedir, cp, strlen(cp));
+ strsave(>u_basedir, cp);
} else if (strncmp(s, "skel_dir", 8) == 0) {
for (cp = s + 8 ; isspace((unsigned char)*cp);
cp++) {
}
- memsave(>u_skeldir, cp, strlen(cp));
+ strsave(>u_skeldir, cp);
} else if (strncmp(s, "shell", 5) == 0) {
for (cp = s + 5 ; isspace((unsigned char)*cp);
cp++) {
}
- memsave(>u_shell, cp, strlen(cp));
+ strsave(>u_shell, cp);
} else if (strncmp(s, "password", 8) == 0) {
for (cp = s + 8 ; isspace((unsigned char)*cp);
cp++) {
}
- memsave(>u_password, cp, strlen(cp));
+ strsave(>u_password, cp);
} else if (strncmp(s, "class", 5) == 0) {
for (cp = s + 5 ; isspace((unsigned char)*cp);
cp++) {
}
- memsave(>u_class, cp, strlen(cp));
+ strsave(>u_class, cp);
} else if (strncmp(s, "inactive", 8) == 0) {
for (cp = s + 8 ; isspace((unsigned char)*cp);
cp++) {
}
@@ -839,7 +837,7 @@ read_defaults(user_t *up)
free(up->u_inactive);
up->u_inactive = NULL;
} else {
- memsave(>u_inactive, cp,
strlen(cp));
+ strsave(>u_inactive, cp);
}
} else if (strncmp(s, "range", 5) == 0) {
for (cp = s + 5 ; isspace((unsigned char)*cp);
cp++) {
@@ -858,7 +856,7 @@ read_defaults(user_t *up)
free(up->u_expire);
up->u_expire = NULL;
} else {
-
Re: smtpd: nits to reduce the diff with -portable
On Mon, 15 May 2023 13:54:35 +0200, Omar Polo wrote: > almost always (cast)var. I've adjusted the spacing in the line I was > touching, grepping for common types I could only find one instance of > a '(long long) src' in envelope.c which I'm not addressing here. OK millert@. It would be nice to get these changes in portable as well to avoid gratuitous differences. - todd
Re: seperate LRO/TSO flags
On Wed, 10 May 2023 20:55:24 +0200, Jan Klemkow wrote: > For tcprecvoffload and ix(4) it's not possible to enable/disable it per > address family. Its just one flag for the hardware. > > For tcpsendoffload its possible, but I won't do that till its necessary. > > Why would you want to differentiate the address families here? I was mostly just curious as I see FreeBSD seems to support this. That made we wonder if there is hardware that only supports offloading for IPv4. - todd
sigsuspend.2: document behavior for discarded signals
The sigsuspend(2) man page doesn't spell out explicitly what happens for signals that are discarded, either as the default action or where the handler is set to SIG_IGN. I think it should. OK? - todd Index: lib/libc/sys/sigsuspend.2 === RCS file: /cvs/src/lib/libc/sys/sigsuspend.2,v retrieving revision 1.14 diff -u -p -u -r1.14 sigsuspend.2 --- lib/libc/sys/sigsuspend.2 29 May 2017 09:40:02 - 1.14 +++ lib/libc/sys/sigsuspend.2 10 May 2023 17:25:44 - @@ -44,12 +44,15 @@ .Fn sigsuspend temporarily changes the blocked signal mask to the set to which .Fa sigmask -points, -and then waits for a signal to arrive; -on return the previous set of masked signals is restored. -The signal mask set -is usually empty to indicate that all +points, then waits for a signal to arrive that either has a handler +installed or that terminates the process. +On return, the previous set of masked signals is restored. +The signal mask set is usually empty to indicate that all signals are to be unblocked for the duration of the call. +Signals that are discarded by default, or that have the handler set to +.Dv SIG_IGN , +will not interrupt +.Nm . .Pp In normal usage, a signal is blocked using .Xr sigprocmask 2
Re: seperate LRO/TSO flags
On Wed, 10 May 2023 19:03:58 +0200, Jan Klemkow wrote: > This diff introduces separate flags for TCP offloading. We split this > into LRO (large receive offloading) and TSO (TCP segmentation > offloading). Thus, we are able to turn it on/off separately. > > For ifconfig(8) we use "tcprecvoffload" and "tcpsendoffload". So, the > user has a better insight of what this features are doing. Is it possible to control these at the address family level? In other words, is it possible to enable "tcprecvoffload" and "tcpsendoffload" for inet but not inet6 or vice versa? - todd
Re: smtpd: nits to reduce the diff with -portable
On Wed, 10 May 2023 09:25:43 +0200, Omar Polo wrote: > I forgot to include one off_t cast since it was in a different > directory and -even if off topic because it's not in portable- instead > of "const"-ify only tz why don't mark as const also the two arrays day > and month? Sure. OK millert@ for this one too. - todd
Re: smtpd: nits to reduce the diff with -portable
On Wed, 10 May 2023 00:55:54 +0200, Omar Polo wrote: > As per subject, here's a few misc nits that would reduce the > difference with -portable. There's some printing of time_t via > casting to long long, some missing includes (even if in tree it builds > nevertheless) and a const for a variable (no idea how it went there in > -portable but it's not wrong so including that too.) OK millert@ - todd
Re: acme-client.conf example: more explicit clue to test with staging server
On Tue, 09 May 2023 21:45:30 +0200, Theo Buehler wrote: > Some expressed concern that it should be done the other way around, > i.e., leave the default at letsencrypt. Perhaps it's indeed better > this way to avoid creating servers with bad certs. OK millert@ for this version - todd
Re: passwd: fix error paths and undefined behaviour
On Mon, 08 May 2023 16:17:51 -, Tobias Stoeckmann wrote: > Turns out that we have yet another possibility to trigger a theoretical > signed integer overflow if pwd_tries is INT_MAX. This one avoids such > situation as well. OK millert@ - todd
Re: cron: better error checking of random values
Now that the random range changes are committed I would like to
revisit this diff.
This fixes two issues with the parsing of random values:
1) Garbage after a random value is now detected. This fixes a bug
in the random range parsing that handles the optional final
number. For example:
~foo* * * * echo invalid
0~59bar * * * * echo invalid
10~%$! * * * * echo invalid
These kind of syntax errors are already detected for normal ranges.
2) Bounds check the high and low numbers in a random range.
Previously, only the final randomized number was bounds-checked
(which is usually too late). The bounds are checked for normal
ranges in set_element() but in the case of random ranges this
is too late. The following invalid entry is now rejected.
0~60 * * * * echo max minute is 59
Whereas before it would work most (but not all!) of the time.
OK?
- todd
Index: usr.sbin/cron/entry.c
===
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 entry.c
--- usr.sbin/cron/entry.c 6 May 2023 23:06:27 - 1.54
+++ usr.sbin/cron/entry.c 6 May 2023 23:36:56 -
@@ -499,12 +499,24 @@ get_range(bitstr_t *bits, int low, int h
/* get the (optional) number following the tilde
*/
ch = get_number(, low, names, ch, file, "/, \t\n");
- if (ch == EOF)
+ if (ch == EOF) {
+ /* no second number, check for valid terminator
+*/
ch = get_char(file);
+ if (!strchr("/, \t\n", ch)) {
+ unget_char(ch, file);
+ return (EOF);
+ }
+ }
if (ch == EOF || num1 > num2) {
unget_char(ch, file);
return (EOF);
}
+
+ /* we must perform the bounds checking ourselves
+*/
+ if (num1 < low || num2 > high)
+ return (EOF);
if (ch == '/') {
/* randomize the step value instead of num1
Re: crontab: support random offsets for steps
There appears to be a strong preference for a syntax like "~/10"
instead of "*/~10". The following diff implements that instead and
also supports random ranges like "1~58/10". When a high and/or low
value is used with a random range, the initial offset is a random
value less than eiter the step value or the difference of the high
and low values (whichever is smaller).
- todd
Index: usr.sbin/cron/crontab.5
===
RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.41
diff -u -p -u -r1.41 crontab.5
--- usr.sbin/cron/crontab.5 18 Apr 2020 17:11:40 - 1.41
+++ usr.sbin/cron/crontab.5 6 May 2023 02:19:40 -
@@ -157,8 +157,7 @@ If either (or both) of the numbers on ei
.Ql ~
are omitted, the appropriate limit (low or high) for the field will be used.
.Pp
-Step values can be used in conjunction with ranges (but not random ranges
-which represent a single number).
+Step values can be used in conjunction with ranges.
Following a range with
.No / Ns Ar number
specifies skips of
@@ -173,6 +172,18 @@ Steps are also permitted after an asteri
.Dq every two hours ,
just use
.Dq */2 .
+A step value after a random range will execute the command at a random
+offset less than the step size.
+For example, to avoid a thundering herd at the top and bottom of the hour,
+.Dq 0~59/30
+.Po
+or simply
+.Dq ~/30
+.Pc
+can be used in the
+.Ar minute
+field to specify that command execution happen twice an hour at
+consistent intervals.
.Pp
An asterisk
.Pq Ql *
Index: usr.sbin/cron/entry.c
===
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 entry.c
--- usr.sbin/cron/entry.c 21 May 2022 01:21:29 - 1.53
+++ usr.sbin/cron/entry.c 5 May 2023 21:39:30 -
@@ -456,10 +456,11 @@ get_range(bitstr_t *bits, int low, int h
/* range = number | number* "~" number* | number "-" number ["/" number]
*/
- int i, num1, num2, num3;
+ int i, num1, num2, num3, rndstep;
num1 = low;
num2 = high;
+ rndstep = 0;
if (ch == '*') {
/* '*' means [low, high] but can still be modified by /step
@@ -497,7 +498,7 @@ get_range(bitstr_t *bits, int low, int h
/* get the (optional) number following the tilde
*/
- ch = get_number(, low, names, ch, file, ", \t\n");
+ ch = get_number(, low, names, ch, file, "/, \t\n");
if (ch == EOF)
ch = get_char(file);
if (ch == EOF || num1 > num2) {
@@ -505,6 +506,13 @@ get_range(bitstr_t *bits, int low, int h
return (EOF);
}
+ if (ch == '/') {
+ /* randomize the step value instead of num1
+*/
+ rndstep = 1;
+ break;
+ }
+
/* get a random number in the interval [num1, num2]
*/
num3 = num1;
@@ -538,6 +546,13 @@ get_range(bitstr_t *bits, int low, int h
ch = get_number(, 0, NULL, ch, file, ", \t\n");
if (ch == EOF || num3 == 0)
return (EOF);
+ if (rndstep) {
+ /*
+* use a random offset smaller than the step size
+* and the difference between high and low values.
+*/
+ num1 += arc4random_uniform(MINIMUM(num3, num2 - num1));
+ }
} else {
/* no step. default==1.
*/
Index: usr.sbin/cron/macros.h
===
RCS file: /cvs/src/usr.sbin/cron/macros.h,v
retrieving revision 1.15
diff -u -p -u -r1.15 macros.h
--- usr.sbin/cron/macros.h 12 Nov 2015 21:12:05 - 1.15
+++ usr.sbin/cron/macros.h 5 May 2023 21:38:19 -
@@ -29,6 +29,8 @@
#defineMAX_TEMPSTR 100 /* obvious */
#defineMAX_UNAME (_PW_NAME_LEN+1)/* max length of
username, should be overkill */
+#defineMINIMUM(a, b) (((a) < (b)) ? (a) : (b))
+
#defineSkip_Blanks(c, f) \
while (c == '\t' || c == ' ') \
c = get_char(f);
Re: passwd: fix error paths and undefined behaviour
On Fri, 05 May 2023 17:05:05 -, Tobias Stoeckmann wrote: > On Fri, May 05, 2023 at 11:00:12AM -0600, Todd C. Miller wrote: > > This looks OK but I'd like to see an error message if waitpid() > > really does fail. How about something like this, which also avoid > > needing the extra variable? > > Yes, looks much better! OK millert@ - todd
Re: passwd: fix error paths and undefined behaviour
On Fri, 05 May 2023 16:46:51 -, Tobias Stoeckmann wrote:
> In getnewpasswd we increment "tries" every time we try to enter a new
> password. The code allows this to be repeated endlessly by defining
> passwordtries to be 0 in /etc/login.conf. But unfortunately we even
> increment the int "tries" if pwd_tries is 0, which eventually would lead
> to a signed integer overflow (and we would stop asking for passwords
> after billion times).
>
> In pwd_check we should close pipe file descriptors if fork fails. Also
> it might be possible that waitpid fails if the root user tries to
> sabotage the own passwd call. Let's just handle the error case here
> as well to avoid accessing undefined content of "res".
This looks OK but I'd like to see an error message if waitpid()
really does fail. How about something like this, which also avoid
needing the extra variable?
- todd
Index: /usr/src/usr.bin/passwd/pwd_check.c
===
RCS file: /cvs/src/usr.bin/passwd/pwd_check.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 pwd_check.c
--- /usr/src/usr.bin/passwd/pwd_check.c 28 Aug 2021 06:46:49 - 1.17
+++ /usr/src/usr.bin/passwd/pwd_check.c 5 May 2023 16:59:20 -
@@ -184,8 +184,10 @@ pwd_check(login_cap_t *lc, char *passwor
/* get the return value from the child */
while (waitpid(child, , 0) == -1) {
- if (errno != EINTR)
- break;
+ if (errno != EINTR) {
+ warn("waitpid");
+ goto out;
+ }
}
if (WIFEXITED(res) && WEXITSTATUS(res) == 0) {
free(checker);
Re: cron: better error checking of random values
On Fri, 05 May 2023 16:13:01 +1000, Mark Jamsek wrote: > I found kn's attempted syntax intuitive though; it feels like a natural > extension of the existing random and step syntax. I also assumed ~/15 > would run every 15 minutes starting with a random minute, and since > discovering it didn't work like that, I've been carrying a simple patch > that allows kn's syntax: > > ~/15random 15 minute intervals in [0, 59] > 1~9/10 random 10 minute intervals in [1,59] It does seem like people prefer the ~/step syntax over my own, which is fine. However, I'm unsure about the x~y/step syntax. Personally, I think it would be more natural to treat x~y/step like x-y/step and just use a random offset in the interval [0,step-1]. Do you have a use-case for the way your diff handles this or did it just follow naturally from the random value being in num1? Also, using the % operator with the random value results in modulo bias, which we would like to avoid. If we use a random offset based on the step interval instead there is no need for the modulus. - todd
Re: cron: better error checking of random values
On Thu, 04 May 2023 21:41:26 -, Klemens Nanni wrote: > On Thu, May 04, 2023 at 03:30:30PM -0600, Todd C. Miller wrote: > > This fixes two issues with the parsing of random values: > > > > 1) A random value with a step is now rejected. For example: > > > > ~/10* * * * echo invalid > > I've ben using ~/10 to randomly distribute four similar tasks so that > they don't start at the same time. > > Is that wrong? I'm fairly certain that doesn't do what you think it does. When I tested it "~/10" behaved the same as "~". The step value is not even parsed. It sounds like what you want is the proposed syntax "*/~10" to use a random offset. - todd
cron: better error checking of random values
This fixes two issues with the parsing of random values:
1) A random value with a step is now rejected. For example:
~/10* * * * echo invalid
0~59/10 * * * * echo invalid
10~/10 * * * * echo invalid
~40/10 * * * * echo invalid
Previously, the '/' would just be discarded.
2) The high and low random bound values are now checked. Previously,
only the randomized number was bounds-checked (which is usually
too late). This is more consistent with the behavior of ranges
(low-high). The following invalid entry is now rejected.
0~60 * * * * echo max minute is 59
Whereas before it would work most (but not all!) of the time.
OK?
- todd
diff -u -p -u -r1.53 entry.c
--- usr.sbin/cron/entry.c 21 May 2022 01:21:29 - 1.53
+++ usr.sbin/cron/entry.c 4 May 2023 21:19:40 -
@@ -498,12 +498,17 @@ get_range(bitstr_t *bits, int low, int h
/* get the (optional) number following the tilde
*/
ch = get_number(, low, names, ch, file, ", \t\n");
- if (ch == EOF)
+ if (ch == EOF) {
+ /* no second number, check for valid terminator
+*/
ch = get_char(file);
- if (ch == EOF || num1 > num2) {
- unget_char(ch, file);
- return (EOF);
+ if (!strchr(", \t\n", ch)) {
+ unget_char(ch, file);
+ return (EOF);
+ }
}
+ if (num1 > num2 || num1 < low || num2 > high)
+ return (EOF);
/* get a random number in the interval [num1, num2]
*/
crontab: remove tmp file on seteuid() failure
Fix a bug introduced in rev 1.86 (fchown removal). Currently, if
the second seteuid(2) were to fail (not really possible) we would
leave a temporary file in the spool dir. This will be ignored by
cron but we still want to clean it up.
- todd
Index: usr.sbin/cron/crontab.c
===
RCS file: /cvs/src/usr.sbin/cron/crontab.c,v
retrieving revision 1.95
diff -u -p -u -r1.95 crontab.c
--- usr.sbin/cron/crontab.c 22 Jun 2021 20:12:17 - 1.95
+++ usr.sbin/cron/crontab.c 4 May 2023 21:08:02 -
@@ -381,6 +381,41 @@ edit_cmd(void)
syslog(LOG_INFO, "(%s) END EDIT (%s)", RealUser, User);
}
+/* Create a temporary file in the spool dir owned by "pw". */
+static FILE *
+spool_mkstemp(char *template)
+{
+ uid_t euid = geteuid();
+ int fd = -1;
+ FILE *fp;
+
+ if (euid != pw->pw_uid) {
+ if (seteuid(pw->pw_uid) == -1) {
+ warn("unable to change uid to %u", pw->pw_uid);
+ goto bad;
+ }
+ }
+ fd = mkstemp(template);
+ if (euid != pw->pw_uid) {
+ if (seteuid(euid) == -1) {
+ warn("unable to change uid to %u", euid);
+ goto bad;
+ }
+ }
+ if (fd == -1 || !(fp = fdopen(fd, "w+"))) {
+ warn("%s", template);
+ goto bad;
+ }
+ return (fp);
+
+bad:
+ if (fd != -1) {
+ close(fd);
+ unlink(template);
+ }
+ return (NULL);
+}
+
/* returns 0 on success
* -1 on syntax error
* -2 on install error
@@ -390,10 +425,9 @@ replace_cmd(void)
{
char n[PATH_MAX], envstr[MAX_ENVSTR];
FILE *tmp;
- int ch, eof, fd;
+ int ch, eof;
int error = 0;
entry *e;
- uid_t euid = geteuid();
time_t now = time(NULL);
char **envp = env_init();
@@ -407,25 +441,8 @@ replace_cmd(void)
warnc(ENAMETOOLONG, "%s/tmp.X", _PATH_CRON_SPOOL);
return (-2);
}
- if (euid != pw->pw_uid) {
- if (seteuid(pw->pw_uid) == -1) {
- warn("unable to change uid to %u", pw->pw_uid);
- return (-2);
- }
- }
- fd = mkstemp(TempFilename);
- if (euid != pw->pw_uid) {
- if (seteuid(euid) == -1) {
- warn("unable to change uid to %u", euid);
- return (-2);
- }
- }
- if (fd == -1 || !(tmp = fdopen(fd, "w+"))) {
- warn("%s", TempFilename);
- if (fd != -1) {
- close(fd);
- unlink(TempFilename);
- }
+ tmp = spool_mkstemp(TempFilename);
+ if (tmp == NULL) {
TempFilename[0] = '\0';
return (-2);
}
Re: crontab: support random offsets for steps
On Thu, 04 May 2023 07:32:18 -0700, Navan Carson wrote: > Any chance the syntax could be: > > ~/20 * * * * command > > To align with how ~ is used currently. That is already a valid syntax, though not a terribly useful one. It currently results in a random number with a step of 20. - todd
crontab: support random offsets for steps
Crontab supports things like "*/20" in the minutes column to run
every 20 minutes. For example, given:
*/20 * * * * echo I am right on time
The job above would run at 0, 20, and 40 minutes of every hours.
job@ asked whether we could support a random offset so that jobs
would not always start at the same time, but still use the same
period (in this example every 20 minutes). This turns out to
be fairly simple.
Below is a small diff to support step intervals with a random offset.
The syntax adds a '~' after the step value. For example:
*/~20 * * * * echo mix it up a bit
will still run every 20 minutes but the initial run will be some
time between 0-19 minutes after the hour (inclusive). Like the
existing random support, the starting offset for an entry is chosen
when the crontab file is first loaded and remains the same unless
the crontab file is modified (and reloaded).
The man page bits are from job@
Opinions? Does the proposed syntax seem OK?
- todd
Index: usr.sbin/cron/crontab.5
===
RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.41
diff -u -p -u -r1.41 crontab.5
--- usr.sbin/cron/crontab.5 18 Apr 2020 17:11:40 - 1.41
+++ usr.sbin/cron/crontab.5 3 May 2023 20:17:31 -
@@ -174,6 +174,15 @@ Steps are also permitted after an asteri
just use
.Dq */2 .
.Pp
+A step value may be preceded with a
+.Ql ~
+character to specify a one-off random wait period before the step cycle begins.
+For example, to avoid a thundering herd at the top and bottom of the hour,
+.Dq */~30
+can be used in the
+.Ar minute
+field to specify command execution happen twice an hour at consistent
intervals.
+.Pp
An asterisk
.Pq Ql *
is short form for a range of all allowed values.
Index: usr.sbin/cron/entry.c
===
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 entry.c
--- usr.sbin/cron/entry.c 21 May 2022 01:21:29 - 1.53
+++ usr.sbin/cron/entry.c 3 May 2023 17:24:47 -
@@ -524,12 +524,22 @@ get_range(bitstr_t *bits, int low, int h
/* check for step size
*/
if (ch == '/') {
+ int rndstep = 0;
+
/* eat the slash
*/
ch = get_char(file);
if (ch == EOF)
return (EOF);
+ /* check for random step size offset. */
+ if (ch == '~') {
+ rndstep = 1;
+ ch = get_char(file);
+ if (ch == EOF)
+ return (EOF);
+ }
+
/* get the step size -- note: we don't pass the
* names here, because the number is not an
* element id, it's a step size. 'low' is
@@ -538,6 +548,11 @@ get_range(bitstr_t *bits, int low, int h
ch = get_number(, 0, NULL, ch, file, ", \t\n");
if (ch == EOF || num3 == 0)
return (EOF);
+
+ if (rndstep) {
+ /* add a random offset less than the step size */
+ num1 += arc4random_uniform(num3);
+ }
} else {
/* no step. default==1.
*/
Re: rpki-client json.c add json_do_string()
On Tue, 02 May 2023 14:13:27 +0200, Claudio Jeker wrote:
> Add a json_do_string() a function to print a JSON string.
> This function does the needed encoding of control chars and escape chars.
> I skipped the optional encoding of the forward slash (/) since this is
> only needed if the json output is embedded in HTML/SGML/XML.
> People putting JSON into such documents need to pass this through an extra
> step.
Unless you can guarantee that other control characters cannot occur
you probably want to output them in unicode hex form. For example,
something like:
default:
const char hex[] = "0123456789abcdef";
if (iscntrl(c)) {
/* Escape control characters like \u */
if (!eb)
eb = fprintf(jsonfh, "\\u00%c%c", hex[c >> 4],
hex[c & 0x0f]) < 0;
}
In this case you probably want to assign c as an unsigned char.
E.g.
while ((c = (unsigned char)*v++) != '\0') {
...
}
Or better yet just declare c as unsigned char instead of int.
- todd
Re: OpenSMTPD: Don't return message body in successfull DNS reports
On Thu, 20 Apr 2023 19:40:49 +0200, Christopher Zimmermann wrote: > delivery success DSNs include the message body if not explicitely > disabled by RET HDRS. > But according to rfc3461 4.3 the body should _only_ be included for > failure DSNs. > > To me it seems more sane to not include the whole message, which may > well be quite large. Make sense to me. OK millert@ - todd
pax: GNU tar base-256 size field support
I recently ran into a problem with busybox tar generating archives
where the size field is base-256 encoded for files larger than 8GB.
Apparently this is a GNU tar extension.
Do we want to support this in pax? Below is an initial diff that
at least produces the correct results when listing the archive. I
have not yet verified that it gets extracted correctly. If there
is interest I will do some more testing.
- todd
Index: bin/pax/gen_subs.c
===
RCS file: /cvs/src/bin/pax/gen_subs.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 gen_subs.c
--- bin/pax/gen_subs.c 26 Aug 2016 05:06:14 - 1.32
+++ bin/pax/gen_subs.c 19 Apr 2023 02:05:19 -
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
#include "pax.h"
#include "extern.h"
@@ -279,9 +280,10 @@ ul_asc(u_long val, char *str, int len, i
/*
* asc_ull()
- * Convert hex/octal character string into a unsigned long long.
- * We do not have to check for overflow! (The headers in all
- * supported formats are not large enough to create an overflow).
+ * Convert hex/octal/base-256 character string into a unsigned long long.
+ * We only have to check for overflow when parsing base-256.
+ * The headers in all supported formats are not large enough to create
+ * an overflow for hex or octal.
* NOTE: strings passed to us are NOT TERMINATED.
* Return:
* unsigned long long value
@@ -296,9 +298,9 @@ asc_ull(char *str, int len, int base)
stop = str + len;
/*
-* skip over leading blanks and zeros
+* skip over leading blanks
*/
- while ((str < stop) && ((*str == ' ') || (*str == '0')))
+ while ((str < stop) && (*str == ' '))
++str;
/*
@@ -316,7 +318,17 @@ asc_ull(char *str, int len, int base)
else
break;
}
+ } else if (*str == '\200') {
+ /* base-256 encoding, GNU tar extension */
+ while (++str < stop) {
+ if (tval + (unsigned char)*str > ULLONG_MAX >> 8) {
+ /* overflow */
+ return(-1);
+ }
+ tval = (tval << 8) + (unsigned char)*str;
+ }
} else {
+ /* octal */
while ((str < stop) && (*str >= '0') && (*str <= '7'))
tval = (tval << 3) + (*str++ - '0');
}
Re: fix iwm/iwx updatechan callbacks
On Wed, 12 Apr 2023 23:27:08 +0200, Stefan Sperling wrote: > The iwm_updatechan and iwx_updatechan callbacks are not reachable > because they were never wired up. Only the iwn driver already has > this callback pointer set as intended. > > With the patch below iwm/iwx should get triggered when an AP switches > between 20MHz and 40/80MHz channel width, as indicated by the > 11n HT-operation information element in beacons. Working fine here with: iwm0 at pci2 dev 0 function 0 "Intel AC 8260" rev 0x3a, msi iwm0: hw rev 0x200, fw ver 36.ca7b901d.0, address xx:xx:xx:xx:xx:xx though I don't think my AP switches channel widths. - todd
Re: OF_getpropstr()
On Sat, 08 Apr 2023 08:48:31 -0600, "Theo de Raadt" wrote:
> Mark Kettenis wrote:
>
> > > +{
> > > + int len;
> > > +
> > > + len = OF_getprop(handle, prop, buf, buflen);
> > > + if (buflen > 0)
> > > + buf[min(len, buflen - 1)] = '\0';
> > > +
> > > + return (len);
> >
>
> I've mailed dlg seperately, but will raise it here also.
>
> If buflen is 0, then why call OF_getprop at all? I doubt this situation
> occurs, but you want to protect against it, ok
>
> Maybe in the end if looks like this:
>
> int len = 0;
> if (buflen > 0) {
> len = OF_getprop(handle, prop, buf, buflen - 1);
> buf[min(len, buflen - 1)] = '\0';
> }
> return (len);
>
> OF_getprop() is now being called with buflen -1, which can avoid one
> extra character of processing effort for a long input string.
I think that will be wrong for the "name" property. From
sys/dev/ofw/fdt.c:OF_getprop
if (len < 0 && strcmp(prop, "name") == 0) {
data = fdt_node_name(node);
if (data) {
len = strlcpy(buf, data, buflen);
...
So passing in buflen is probably correct.
- todd
Re: csh.1 markup fix
On Thu, 30 Mar 2023 17:26:17 +0200, Omar Polo wrote: > noticed because it renders oddly: > >kill %job >kill[-s signal_name] pid >kill -sig pid ... > > The Op on its own line becomes part of the item body instead of the > item itself. Use an in-line Oo ... Oc instead, ok? OK millert@ - todd
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On Tue, 28 Mar 2023 16:19:42 +0200, Omar Polo wrote: > sigh... forgot to advance the pointer after strrchr otherwise argv[0] > would have been /ksh instead of "ksh". OK millert@ for this version. - todd
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On Mon, 27 Mar 2023 20:06:30 +0200, Omar Polo wrote:
> Is _PATH_BSHELL portable though? I can see a few stuff that uses it
> (vi among others) but I'm not sure.
The paths.h header is a BSD invention, though it may be present on
some other systems. I don't think that's a reason not to use it
though.
> Also, if you look at the callers of shellcmdoutput() they all prepare
> the argv array as {"sh", "-c", "command provided", NULL} so I'm
> wondering if we should just ignore $SHELL and always use /bin/sh, or
> change that "sh" accordingly to $SHELL.
It might be best to use the basename of the actual shell for argv[0].
Our ksh for instance has slightly different behavior when invoked
as sh.
OK millert@ for the diff as-is.
- todd
> Index: region.c
> ===
> RCS file: /cvs/src/usr.bin/mg/region.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 region.c
> --- region.c 27 Mar 2023 17:54:20 - 1.42
> +++ region.c 27 Mar 2023 17:58:11 -
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -483,7 +484,8 @@ shellcmdoutput(char* const argv[], char*
> return (FALSE);
> }
>
> - shellp = getenv("SHELL");
> + if ((shellp = getenv("SHELL")) == NULL)
> + shellp = _PATH_BSHELL;
>
> ret = pipeio(shellp, argv, text, len, bp);
>
> @@ -529,8 +531,6 @@ pipeio(const char* const path, char* con
> if (dup2(s[1], STDOUT_FILENO) == -1)
> _exit(1);
> if (dup2(s[1], STDERR_FILENO) == -1)
> - _exit(1);
> - if (path == NULL)
> _exit(1);
>
> execv(path, argv);
>
Re: recv.c: consistency wrt NULL for pointers
On Sat, 25 Mar 2023 20:13:35 +0100, Otto Moerbeek wrote: > Last arg also is a pointer, so pass NULL. Looks like it's been that way since the initial CSRG commit. OK millert@ - todd
smtpd: simplify token name extraction for %{name}
The current code for extracting the token name from %{name} can be
simplified by computing the token name length. The existing code
copies "name}" to token[] using memcpy(), then strchr() to find the
'}' and replace it with a NUL. Using strchr() here is fragile since
token[] is not yet NUL-terminated. This is currently not a problem
since there is an earlier check for '}' in the source string but
it could be dangerous is the code changes further.
I find it much simpler to compute the token name length, verify the
length, copy the bytes and then explicitly NUL-terminate token.
This results in less code and is more easily audited.
I've also removed the duplicate check for *(pbuf+1) != '{'.
OK?
- todd
Index: usr.sbin/smtpd/mda_variables.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mda_variables.c,v
retrieving revision 1.8
diff -u -p -U10 -u -r1.8 mda_variables.c
--- usr.sbin/smtpd/mda_variables.c 19 Mar 2023 01:43:11 - 1.8
+++ usr.sbin/smtpd/mda_variables.c 19 Mar 2023 13:59:01 -
@@ -236,21 +236,21 @@ mda_expand_token(char *dest, size_t len,
ssize_t
mda_expand_format(char *buf, size_t len, const struct deliver *dlv,
const struct userinfo *ui, const char *mda_command)
{
chartmpbuf[EXPAND_BUFFER], *ptmp, *pbuf, *ebuf;
charexptok[EXPAND_BUFFER];
ssize_t exptoklen;
chartoken[MAXTOKENLEN];
- size_t ret, tmpret;
+ size_t ret, tmpret, toklen;
if (len < sizeof tmpbuf) {
log_warnx("mda_expand_format: tmp buffer < rule buffer");
return -1;
}
memset(tmpbuf, 0, sizeof tmpbuf);
pbuf = buf;
ptmp = tmpbuf;
ret = tmpret = 0;
@@ -261,48 +261,46 @@ mda_expand_format(char *buf, size_t len,
if (tmpret >= sizeof tmpbuf) {
log_warnx("warn: user directory for %s too large",
ui->directory);
return 0;
}
ret += tmpret;
ptmp += tmpret;
pbuf += 2;
}
-
/* expansion loop */
for (; *pbuf && ret < sizeof tmpbuf; ret += tmpret) {
if (*pbuf == '%' && *(pbuf + 1) == '%') {
*ptmp++ = *pbuf++;
pbuf += 1;
tmpret = 1;
continue;
}
if (*pbuf != '%' || *(pbuf + 1) != '{') {
*ptmp++ = *pbuf++;
tmpret = 1;
continue;
}
/* %{...} otherwise fail */
- if (*(pbuf+1) != '{' || (ebuf = strchr(pbuf+1, '}')) == NULL)
+ if ((ebuf = strchr(pbuf+2, '}')) == NULL)
return 0;
/* extract token from %{token} */
- if ((size_t)(ebuf - pbuf) - 1 >= sizeof token)
+ toklen = ebuf - (pbuf+2);
+ if (toklen >= sizeof token)
return 0;
- memcpy(token, pbuf+2, ebuf-pbuf-1);
- if (strchr(token, '}') == NULL)
- return 0;
- *strchr(token, '}') = '\0';
+ memcpy(token, pbuf+2, toklen);
+ token[toklen] = '\0';
exptoklen = mda_expand_token(exptok, sizeof exptok, token, dlv,
ui, mda_command);
if (exptoklen == -1)
return -1;
/* writing expanded token at ptmp will overflow tmpbuf */
if (sizeof (tmpbuf) - (ptmp - tmpbuf) <= (size_t)exptoklen)
return -1;
Re: syslogd udp remote logging eacces
On Thu, 16 Mar 2023 18:52:23 +0100, Alexander Bluhm wrote: > When syslogd is sending messages per UDP, it stops if there is a > permanent error. It has a list of transient errors. > > Since OpenBSD 6.5 pf has changed its error code to EACCES. If pf > blocks your outgoing syslog packets and your fix pf.conf, syslogd > still does not send messages to remote syslog server. > > Better continue trying also in that case. It restores pre-6.5 > behavior. OK millert@ - todd
Re: ssh nits
On Wed, 08 Mar 2023 09:02:08 -0600, joshua stein wrote:
> In the non-fail case, done is set to NULL and then free()d.
> free(NULL) is legal but maybe worth removing?
Please leave this as-is. I don't think it is worth appeasing
cppcheck in this case.
> diff --git usr.bin/ssh/scp.c usr.bin/ssh/scp.c
> index f0f09bba623..acb7bd8a8a1 100644
> --- usr.bin/ssh/scp.c
> +++ usr.bin/ssh/scp.c
> @@ -935,19 +935,21 @@ brace_expand(const char *pattern, char ***patternsp, si
> ze_t *npatternsp)
> *npatternsp = ndone;
> done = NULL;
> ndone = 0;
> ret = 0;
> fail:
> for (i = 0; i < nactive; i++)
> free(active[i]);
> free(active);
> - for (i = 0; i < ndone; i++)
> - free(done[i]);
> - free(done);
> + if (done) {
> + for (i = 0; i < ndone; i++)
> + free(done[i]);
> + free(done);
> + }
> return ret;
> }
>
> static struct sftp_conn *
> do_sftp_connect(char *host, char *user, int port, char *sftp_direct,
> int *reminp, int *remoutp, int *pidp)
> {
> if (sftp_direct == NULL) {
>
>
> grp == NULL fatal()s, so remove the ternary operations that will
> never be the conditionals they aspire to be.
OK
> diff --git usr.bin/ssh/sshpty.c usr.bin/ssh/sshpty.c
> index faf8960cfb5..690263a8cf3 100644
> --- usr.bin/ssh/sshpty.c
> +++ usr.bin/ssh/sshpty.c
> @@ -143,8 +143,8 @@ pty_setowner(struct passwd *pw, const char *tty)
> grp = getgrnam("tty");
> if (grp == NULL)
> fatal("no tty group");
> - gid = (grp != NULL) ? grp->gr_gid : pw->pw_gid;
> - mode = (grp != NULL) ? 0620 : 0600;
> + gid = grp->gr_gid;
> + mode = 0620;
>
> /*
>* Change owner and mode of the tty as required.
>
>
> These parentheses checking the result of an assignment were
> confusing, so move them.
OK. This will result in encode_dest_constraint() and
parse_dest_constraint() returning an SSH_ERR_* instead of 1 on error
but that seems like an improvement. It won't affect the caller's
logic as far as I can tell. I would wait for an OK from djm@ though.
> diff --git usr.bin/ssh/authfd.c usr.bin/ssh/authfd.c
> index 4b81b385637..05011f8c5c9 100644
> --- usr.bin/ssh/authfd.c
> +++ usr.bin/ssh/authfd.c
> @@ -489,8 +489,8 @@ encode_dest_constraint(struct sshbuf *m, const struct des
> t_constraint *dc)
>
> if ((b = sshbuf_new()) == NULL)
> return SSH_ERR_ALLOC_FAIL;
> - if ((r = encode_dest_constraint_hop(b, >from) != 0) ||
> - (r = encode_dest_constraint_hop(b, >to) != 0) ||
> + if ((r = encode_dest_constraint_hop(b, >from)) != 0 ||
> + (r = encode_dest_constraint_hop(b, >to)) != 0 ||
> (r = sshbuf_put_string(b, NULL, 0)) != 0) /* reserved */
> goto out;
> if ((r = sshbuf_put_stringb(m, b)) != 0)
> diff --git usr.bin/ssh/readconf.c usr.bin/ssh/readconf.c
> index e9d3a756896..81456c9b6d3 100644
> --- usr.bin/ssh/readconf.c
> +++ usr.bin/ssh/readconf.c
> @@ -602,7 +602,7 @@ match_cfg_line(Options *options, char **condition, struct
> passwd *pw,
> }
> arg = criteria = NULL;
> this_result = 1;
> - if ((negate = attrib[0] == '!'))
> + if ((negate = (attrib[0] == '!')))
> attrib++;
> /* Criterion "all" has no argument and must appear alone */
> if (strcasecmp(attrib, "all") == 0) {
> diff --git usr.bin/ssh/ssh-agent.c usr.bin/ssh/ssh-agent.c
> index 0e4d7f675ab..de1cdb049a2 100644
> --- usr.bin/ssh/ssh-agent.c
> +++ usr.bin/ssh/ssh-agent.c
> @@ -1010,8 +1010,8 @@ parse_dest_constraint(struct sshbuf *m, struct dest_con
> straint *dc)
> error_fr(r, "parse");
> goto out;
> }
> - if ((r = parse_dest_constraint_hop(frombuf, >from) != 0) ||
> - (r = parse_dest_constraint_hop(tobuf, >to) != 0))
> + if ((r = parse_dest_constraint_hop(frombuf, >from)) != 0 ||
> + (r = parse_dest_constraint_hop(tobuf, >to)) != 0)
> goto out; /* already logged */
> if (elen != 0) {
> error_f("unsupported extensions (len %zu)", elen);
Re: ps(1): fix command alignment
On Wed, 08 Mar 2023 01:37:18 +0100, Tobias Heider wrote: > It look like this was broken in 1.83 which introduces print_comm_name() but > wrongly assumes the returned value is the length difference when it actually > is the updated length value. With this fixed I get a correctly aligned output OK millert@ - todd
Re: mountd: no need for critical sections
On Thu, 02 Mar 2023 07:25:17 +, Klemens Nanni wrote: > The TERM handler also just sets a flag today, but etc/rc.d/mountd still > has `rc_stop=NO' since 2013 > > Do not allow stopping/restarting mountd using the rc.d(8) framework; > if there is need to send a SIGTERM to mountd(8), it should be done > manually as there is too much involved with RPC daemons to make it > automagic. > > Can this be flipped so `rcctl stop mountd' works again? I don't think anything has changed in that respect. However, I'm not sure why this was disabled in the first place. - todd
mountd: no need for critical sections
The SIGHUP handler only sets a flag these days, there is no longer
any need to block it while using the exports list.
OK?
- todd
Index: sbin/mountd/mountd.c
===
RCS file: /cvs/src/sbin/mountd/mountd.c,v
retrieving revision 1.90
diff -u -p -u -r1.90 mountd.c
--- sbin/mountd/mountd.c1 Mar 2023 23:27:46 - 1.90
+++ sbin/mountd/mountd.c1 Mar 2023 23:29:18 -
@@ -736,7 +736,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
char rpcpath[RPCMNT_PATHLEN+1], dirpath[PATH_MAX];
struct hostent *hp = NULL;
struct exportlist *ep;
- sigset_t sighup_mask;
int defset, hostset;
struct fhreturn fhr;
struct dirlist *dp;
@@ -746,8 +745,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
u_short sport;
long bad = 0;
- sigemptyset(_mask);
- sigaddset(_mask, SIGHUP);
saddr = transp->xp_raddr.sin_addr.s_addr;
sport = ntohs(transp->xp_raddr.sin_port);
switch (rqstp->rq_proc) {
@@ -792,7 +789,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
}
/* Check in the exports list */
- sigprocmask(SIG_BLOCK, _mask, NULL);
ep = bad ? NULL : ex_search(_fsid);
hostset = defset = 0;
if (ep && (chk_host(ep->ex_defdir, saddr, , ) ||
@@ -804,7 +800,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
if (!svc_sendreply(transp, xdr_long,
(caddr_t)))
syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
return;
}
if (hostset & DP_HOSTSET)
@@ -820,7 +815,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
if (!svc_sendreply(transp, xdr_long,
(caddr_t)))
syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
return;
}
if (!svc_sendreply(transp, xdr_fhs, (caddr_t)))
@@ -844,7 +838,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
if (bad && !svc_sendreply(transp, xdr_long, (caddr_t)))
syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
return;
case RPCMNT_DUMP:
if (!svc_sendreply(transp, xdr_mlist, NULL))
@@ -958,11 +951,7 @@ xdr_explist(XDR *xdrsp, caddr_t cp)
{
struct exportlist *ep;
int false = 0, putdef;
- sigset_t sighup_mask;
- sigemptyset(_mask);
- sigaddset(_mask, SIGHUP);
- sigprocmask(SIG_BLOCK, _mask, NULL);
ep = exphead;
while (ep) {
putdef = 0;
@@ -973,12 +962,10 @@ xdr_explist(XDR *xdrsp, caddr_t cp)
goto errout;
ep = ep->ex_next;
}
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
if (!xdr_bool(xdrsp, ))
return (0);
return (1);
errout:
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
return (0);
}
Re: mountd: potential use of uninitialized stack data
On Wed, 22 Feb 2023 17:11:47 +0100, Moritz Buhl wrote:
> The tool finds a path to ex_search where fsb.f_fsid is uninitialized.
>
> ex_search compares the potentially uninitialized stack data:
>
> ex_search(fsid_t *fsid)
> {
> struct exportlist *ep;
>
> ep = exphead;
> while (ep) {
> if (ep->ex_fs.val[0] == fsid->val[0] &&
> ...
>
> Is it sufficient to zero fsb?
> Is this really reachable?
I believe that it is. However, I don't know why we wouldn't just
want to skip ex_search, etc when bad is set. Also, there is no
reason to use a critical section now that the SIGHUP handle just
sets a flag. Something like this (untested), should be fine.
It needs testing by someone who actually uses NFS though...
- todd
Index: sbin/mountd/mountd.c
===
RCS file: /cvs/src/sbin/mountd/mountd.c,v
retrieving revision 1.89
diff -u -p -u -r1.89 mountd.c
--- sbin/mountd/mountd.c6 Aug 2020 17:57:32 - 1.89
+++ sbin/mountd/mountd.c22 Feb 2023 17:17:44 -
@@ -736,7 +736,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
char rpcpath[RPCMNT_PATHLEN+1], dirpath[PATH_MAX];
struct hostent *hp = NULL;
struct exportlist *ep;
- sigset_t sighup_mask;
int defset, hostset;
struct fhreturn fhr;
struct dirlist *dp;
@@ -746,8 +745,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
u_short sport;
long bad = 0;
- sigemptyset(_mask);
- sigaddset(_mask, SIGHUP);
saddr = transp->xp_raddr.sin_addr.s_addr;
sport = ntohs(transp->xp_raddr.sin_port);
switch (rqstp->rq_proc) {
@@ -790,9 +787,10 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
fprintf(stderr, "stat failed on %s\n", dirpath);
bad = ENOENT; /* We will send error reply later */
}
+ if (bad)
+ goto mount_error;
/* Check in the exports list */
- sigprocmask(SIG_BLOCK, _mask, NULL);
ep = ex_search(_fsid);
hostset = defset = 0;
if (ep && (chk_host(ep->ex_defdir, saddr, , ) ||
@@ -800,13 +798,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
chk_host(dp, saddr, , )) ||
(defset && scan_tree(ep->ex_defdir, saddr) == 0 &&
scan_tree(ep->ex_dirl, saddr) == 0))) {
- if (bad) {
- if (!svc_sendreply(transp, xdr_long,
- (caddr_t)))
- syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
- return;
- }
if (hostset & DP_HOSTSET)
fhr.fhr_flag = hostset;
else
@@ -817,11 +808,7 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
if (imsg_getfh(dirpath, (fhandle_t *)_fh) < 0) {
bad = errno;
syslog(LOG_ERR, "Can't get fh for %s", dirpath);
- if (!svc_sendreply(transp, xdr_long,
- (caddr_t)))
- syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
- return;
+ goto mount_error;
}
if (!svc_sendreply(transp, xdr_fhs, (caddr_t)))
syslog(LOG_ERR, "Can't send reply");
@@ -842,9 +829,11 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
} else
bad = EACCES;
- if (bad && !svc_sendreply(transp, xdr_long, (caddr_t)))
- syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
+ if (bad) {
+ mount_error:
+ if (!svc_sendreply(transp, xdr_long, (caddr_t)))
+ syslog(LOG_ERR, "Can't send reply");
+ }
return;
case RPCMNT_DUMP:
if (!svc_sendreply(transp, xdr_mlist, NULL))
@@ -958,11 +947,7 @@ xdr_explist(XDR *xdrsp, caddr_t cp)
{
struct exportlist *ep;
int false = 0, putdef;
- sigset_t sighup_mask;
- sigemptyset(_mask);
- sigaddset(_mask, SIGHUP);
- sigprocmask(SIG_BLOCK, _mask, NULL);
ep = exphead;
while (ep) {
putdef = 0;
@@ -973,12 +958,9 @@ xdr_explist(XDR *xdrsp, caddr_t cp)
goto errout;
ep = ep->ex_next;
}
- sigprocmask(SIG_UNBLOCK, _mask, NULL);
- if (!xdr_bool(xdrsp, ))
- return (0);
- return (1);
+
Re: llvm-strip vs ld.bfd (at least on i386): SIGABRT in sys_execve
On Wed, 15 Feb 2023 09:03:55 -0700, "Todd C. Miller" wrote: > It should not be removing .shstrtab. What happens if you tell > llvm-strip to preserve .shstrtab? E.g. --keep-section .shstrtab? Nevermind, I misread the readelf output, the stripped binary does actually have .shstrtab. - todd
Re: llvm-strip vs ld.bfd (at least on i386): SIGABRT in sys_execve
It should not be removing .shstrtab. What happens if you tell llvm-strip to preserve .shstrtab? E.g. --keep-section .shstrtab? - todd
Re: Update share/locale/ctype/en_US.UTF-8.src to Unicode 14.0.0
On Tue, 14 Feb 2023 17:47:00 -0800, Andrew Hewus Fresh wrote: > With the perl update, we get a new version of unicode available to > update this file as well. This was just running the script with the new > perl version. OK millert@ - todd
Re: remove reference to auth_getchallenge
On Sun, 05 Feb 2023 10:43:58 -0500, aisha wrote: > The auth_getchallenge function doesn't seem to exist in the code base. > OK to remove this reference? Yes, that should be removed. OK millert@ - todd
Re: strptime.c
Unfortunately we cannot use strtonum(3) here since there may be
non-digit characters following the number. So, strtoll(3)
it is then.
- todd
Index: lib/libc/time/strptime.c
===
RCS file: /cvs/src/lib/libc/time/strptime.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 strptime.c
--- lib/libc/time/strptime.c12 May 2019 12:49:52 - 1.30
+++ lib/libc/time/strptime.c29 Jan 2023 15:13:53 -
@@ -29,8 +29,10 @@
*/
#include
+#include
+#include
#include
-#include
+#include
#include
#include
@@ -72,8 +74,8 @@ static const int mon_lengths[2][MONSPERY
{ 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }
};
-static int _conv_num64(const unsigned char **, int64_t *, int64_t, int64_t);
static int _conv_num(const unsigned char **, int *, int, int);
+static int epoch_to_tm(const unsigned char **, struct tm *);
static int leaps_thru_end_of(const int y);
static char *_strptime(const char *, const char *, struct tm *, int);
static const u_char *_find_string(const u_char *, int *, const char * const *,
@@ -338,15 +340,10 @@ literal:
if (!(_conv_num(, >tm_sec, 0, 60)))
return (NULL);
break;
- case 's': /* Seconds since epoch */
- {
- int64_t i64;
- if (!(_conv_num64(, , 0, INT64_MAX)))
- return (NULL);
- if (!gmtime_r(, tm))
- return (NULL);
- fields = 0x; /* everything */
- }
+ case 's': /* Seconds since epoch. */
+ if (!(epoch_to_tm(, tm)))
+ return (NULL);
+ fields = 0x; /* everything */
break;
case 'U': /* The week of year, beginning on sunday. */
case 'W': /* The week of year, beginning on monday. */
@@ -610,26 +607,27 @@ _conv_num(const unsigned char **buf, int
}
static int
-_conv_num64(const unsigned char **buf, int64_t *dest, int64_t llim, int64_t
ulim)
+epoch_to_tm(const unsigned char **buf, struct tm *tm)
{
- int result = 0;
- int64_t rulim = ulim;
-
- if (**buf < '0' || **buf > '9')
- return (0);
-
- /* we use rulim to break out of the loop when we run out of digits */
- do {
- result *= 10;
- result += *(*buf)++ - '0';
- rulim /= 10;
- } while ((result * 10 <= ulim) && rulim && **buf >= '0' && **buf <=
'9');
-
- if (result < llim || result > ulim)
- return (0);
-
- *dest = result;
- return (1);
+ int saved_errno = errno;
+ int ret = 0;
+ time_t secs;
+ char *ep;
+
+ errno = 0;
+ secs = strtoll(*buf, , 10);
+ if (*buf == (unsigned char *)ep)
+ goto done;
+ if (secs < 0 ||
+ secs == LLONG_MAX && errno == ERANGE)
+ goto done;
+ if (localtime_r(, tm) == NULL)
+ goto done;
+ ret = 1;
+done:
+ *buf = ep;
+ errno = saved_errno;
+ return (ret);
}
static const u_char *
Re: strptime.c
On Thu, 26 Jan 2023 10:29:36 -0800, enh wrote: > yeah, but that's the copy & paste-o, no? (apologies if it's just too early > for me to be looking at code yet...) > > doesn't this need to be int64_t? > > int result = 0; Yes, it does, thanks. > https://github.com/openbsd/src/blob/master/lib/libc/time/strptime.c#L613 > > (i think the overflow checks are insufficient in both copies of the > function too, especially around the min and max values, but this seems like > the biggest problem with the code.) The checks for underflow/overflow appear to be insufficient. My inclination is to just convert everything to use strtonum(3). - todd
Re: strptime.c
On Thu, 26 Jan 2023 09:36:52 -0800, enh wrote: > it's quite possible that this could use _conv_num64(), but it wasn't > obvious to me that that function's correct? (i haven't thought too hard > about the overflow logic, but just the fact that result is an `int` seems > odd?) It just returns a boolean value, 1 for OK, 0 for not OK. There is a result parameter for the output value. The code is effectively the same as _conv_num. I dislike that it uses int64_t instead of time_t though. - todd
Re: strptime.c
Is there a reason you didn't just change the gmtime_r() in the 's' case to localtime_r()? That seems like the simplest fix. Using strtol() for what may be a 64-bit value on an 32-bit system looks wrong. - todd
Re: mem.4: be more accurate about securelevel
On Fri, 20 Jan 2023 11:29:15 -0700, "Theo de Raadt" wrote: > During this mimmmutable and xonly work, I keep finding test machines where > I enabled kern.allowkmem, and have to disable it. Sometimes weeks later. > Both kern.allowkmem and securelevel disabling are dangerous, especially in > our world where so much other dangerous stuff has been stopped. I wonder if it makes sense to have a version of sysctl.conf that only gets used for the next reboot and then is removed, kind of like /etc/rc.firsttime. Maybe call it /etc/sysctl.once. - todd
Re: Inconsistent isdigit(3) man page
On Fri, 20 Jan 2023 09:32:38 -0700, Bob Beck wrote: > So isdigit(3) says in the first paragraph that > > 'The complete list of decimal digits is 0 and 1-9, in any locale.' > > Later on it says: > > 'On systems supporting non-ASCII single-byte character encodings, > different c arguments may correspond to the digits, and the results of > isdigit() may depend on the LC_CTYPE locale(1).' > > Argubly worring about non ASCII single byte character encodings > doesn't make sense for an OpenBSD man page, but setting that aside for > a minute it's the second part that is of concern.. "It may depend on > the LC_CTYPE locale()" - Ok, well does it or doesn't it? On OpenBSD isdigit() never uses the locale, but you already knew that. > Various spec docs seem all over the place on this, so I am also > paging Dr. Posix in this email... Hi Philip! :) Is isdigit() > safe from being screwed up by locale or not? POSIX says that isdigit() may be influenced by the locale. Since LC_CTYPE defines the different character classes, of which "digit" is one, I think that part of the manual is correct as-is. However, perhaps we should make isdigit(3) match isalpha(3) like so: Index: lib/libc/gen/isdigit.3 === RCS file: /cvs/src/lib/libc/gen/isdigit.3,v retrieving revision 1.13 diff -u -p -u -r1.13 isdigit.3 --- lib/libc/gen/isdigit.3 11 Sep 2022 06:38:10 - 1.13 +++ lib/libc/gen/isdigit.3 20 Jan 2023 16:58:20 - @@ -51,7 +51,12 @@ The and .Fn isdigit_l functions test for any decimal-digit character. -The complete list of decimal digits is 0 and 1\(en9, in any locale. +In the C locale, the complete list of decimal digits is 0 and 1\(en9. +.Ox +always uses the C locale for these functions, +ignoring the global locale, the thread-specific locale, and the +.Fa locale +argument. .Sh RETURN VALUES These functions return zero if the character tests false or non-zero if the character tests true.
Re: Remove remnants of removed diff -l option
On Wed, 04 Jan 2023 13:13:28 -0800, Nathan Houghton wrote: > This patch removes a few remnants of the unsupported diff -l > option from diff.c and the diff manual page. > > The diff.c usage() print is already correct, so no changes are > needed there. Thanks, committed. - todd
Re: [PATCH] Correctly (per POSIX) round up df usage percentage
On Mon, 29 Aug 2022 13:51:13 +0200, =?utf-8?B?0L3QsNCx?= wrote: > In that case, how about this scissor-patch? > It has the added benefit of removing the existing floating-point usage. That version looks good to me, committed. - todd
Re: df.1: document that -P disables BLOCKSIZE
Updated version. I kept "The BLOCKSIZE environment variable" in the -P description since it is the first time the man page metions BLOCKSIZE. - todd Index: bin/df/df.1 === RCS file: /cvs/src/bin/df/df.1,v retrieving revision 1.48 diff -u -p -u -r1.48 df.1 --- bin/df/df.1 10 Aug 2016 19:46:43 - 1.48 +++ bin/df/df.1 31 Dec 2022 21:29:50 - @@ -100,6 +100,9 @@ with the possibly stale statistics that .It Fl P Print out information in a stricter format designed to be parsed by portable scripts. +The +.Ev BLOCKSIZE +environment variable is ignored when this option is specified. .It Fl t Ar type Indicate the actions should only be taken on file systems of the specified @@ -125,14 +128,13 @@ the last option given overrides the othe .Sh ENVIRONMENT .Bl -tag -width BLOCKSIZE .It Ev BLOCKSIZE -If the environment variable -.Ev BLOCKSIZE -is set, and the -.Fl h +Display block counts in units of size +.Dv BLOCKSIZE . +Ignored if any of the +.Fl h , k or -.Fl k -options are not specified, the block counts will be displayed in units of that -size block. +.Fl P +options are specified. .El .Sh EXIT STATUS .Ex -std df
Re: ssh: progress meter: prefer setitimer(2) to alarm(3)
On Sat, 31 Dec 2022 19:05:20 +0100, Mark Kettenis wrote: > Bad idea. The setitimer(2) interface is marked as "OB XSI" in POSIX, > which means that it is considerent "Obsolescent" and may be removed in > a future version of POSIX. Since we want ssh to be as portable as > possible we shouldn't use it there. Especially for something that > really is just a cosmetic "fix". I considered this but I can't think of any platform that OpenSSH supports which doesn't have setitimer(2). The reality is that even if POSIX removes setitimer(2), systems that already implement it will not remove it. So I think the only concern is some theoretical future POSIX system. - todd
df.1: document that -P disables BLOCKSIZE
In POSIX mode, df(1) does not honor the BLOCKSIZE environment variable. Any comments on the wording? - todd Index: bin/df/df.1 === RCS file: /cvs/src/bin/df/df.1,v retrieving revision 1.48 diff -u -p -u -r1.48 df.1 --- bin/df/df.1 10 Aug 2016 19:46:43 - 1.48 +++ bin/df/df.1 31 Dec 2022 17:50:39 - @@ -100,6 +100,9 @@ with the possibly stale statistics that .It Fl P Print out information in a stricter format designed to be parsed by portable scripts. +The +.Ev BLOCKSIZE +environment variable is ignored when this option is specified. .It Fl t Ar type Indicate the actions should only be taken on file systems of the specified @@ -127,11 +130,12 @@ the last option given overrides the othe .It Ev BLOCKSIZE If the environment variable .Ev BLOCKSIZE -is set, and the -.Fl h +is set, and neither of the +.Fl h , +.Fl k , or -.Fl k -options are not specified, the block counts will be displayed in units of that +.Fl P +options are specified, the block counts will be displayed in units of that size block. .El .Sh EXIT STATUS
Re: ssh: progress meter: prefer setitimer(2) to alarm(3)
On Sat, 31 Dec 2022 10:33:26 -0500, Scott Cheloha wrote: > The progress meter in scp(1) and sftp(1) updates periodically, once > per second. But using alarm(3) to repeatedly rearm the signal causes > that update period to drift forward: OK millert@ - todd
