rc(8): reorder_libs(): print names of relinked libraries

2022-07-29 Thread Scott Cheloha
Recently I've been doing some MIPS64 stuff on my EdgeRouter PoE.  It
has a USB disk, two 500MHz processors, and 512MB of RAM.

So, every time I reboot to test the next iteration of my kernel
patch, I get to here:

reordering libraries: 

and I sit there for half a minute or more and wonder what the hell
it's doing.

And, in my intellectual brain, I know it's relinking the libraries
and that this is slow because it needs to link a bunch of object files
and my machine is slow and my disk is slow and I have almost no RAM.

But!  My animal brain wishes I could see some indication of progress.
Because the script has told me it is linking more than one library.
So, as with daemon startup, I am curious which library it is working
on at any given moment.

Can we print the library names as they are being relinked?

With the attached patch the boot now looks like this:

reordering libraries: ld.so libc.so.96.1 libcrypto.so.49.1.

We print the library name before it is relinked, so you can know which
library it is linking.

If for some reason we fail on a particular library, it instead looks
like this:

reordering libraries: ld.so(failed).

... which is me trying to imitate what we do for daemon startup.

Thoughts?

I know this makes rc(8) a bit noisier but it really does improve my
(for want of a better term) "user experience" as I wait for my machine
to boot.

Index: rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.563
diff -u -p -r1.563 rc
--- rc  28 Jul 2022 16:06:04 -  1.563
+++ rc  30 Jul 2022 00:15:26 -
@@ -193,7 +193,7 @@ reorder_libs() {
# Remount the (read-only) filesystems in _ro_list as read-write.
for _mp in $_ro_list; do
if ! mount -u -w $_mp; then
-   echo ' failed.'
+   echo '(failed).'
return
fi
done
@@ -210,6 +210,7 @@ reorder_libs() {
_install='install -F -o root -g bin -m 0444'
_lib=${_liba##*/}
_lib=${_lib%.a}
+   echo -n " $_lib"
_lib_dir=${_liba#$_relink}
_lib_dir=${_lib_dir%/*}
cd $_tmpdir
@@ -243,9 +244,9 @@ reorder_libs() {
done
 
if $_error; then
-   echo ' failed.'
+   echo '(failed).'
else
-   echo ' done.'
+   echo '.'
fi
 }
 



Re: remove pre-486 code from i386 platform

2022-07-29 Thread Tomasz Rola
On Fri, Jul 29, 2022 at 02:46:26AM -0400, Daniel Dickman wrote:
> 
[...]
> 
> Should we add defines for the 286 too?

I do not think OpenBSD ever ran on 286, right? Adding defines for 286
would thus be rather pointless.

Keeping some #define lines is easier than explaining every now and
then why numbers defined start from, say, 3 or 5, why 0 should not be
redefined and so on - well I do not know, maybe it should be
reused. There will be more such unsupported architectures and cpus and
more defines will become obsolete. So I personally would have kept
those lines and not have to explain or keep saying "read it in the
docs".

At the same time, ok, perhaps nobody will really care about such
details. I might be wrong about it.

-- 
Regards,
Tomasz Rola

--
** A C programmer asked whether computer had Buddha's nature.  **
** As the answer, master did "rm -rif" on the programmer's home**
** directory. And then the C programmer became enlightened...  **
** **
** Tomasz Rola  mailto:tomasz_r...@bigfoot.com **



Build llvm-cov in base

2022-07-29 Thread Frederic Cambus
Hi tech@,

Now that we have llvm-profdata in base, I would like to propose adding
llvm-cov as well. Just like llvm-profdata, it is fast to build and
only takes a few seconds on my amd64 machine.

Having it in base would allow producing reports from coverage data
processed with llvm-profdata without having to install the devel/llvm
port.

Comments? OK?

Index: gnu/usr.bin/clang/Makefile
===
RCS file: /cvs/src/gnu/usr.bin/clang/Makefile,v
retrieving revision 1.25
diff -u -p -r1.25 Makefile
--- gnu/usr.bin/clang/Makefile  26 Jul 2022 15:37:34 -  1.25
+++ gnu/usr.bin/clang/Makefile  29 Jul 2022 20:21:18 -
@@ -108,6 +108,7 @@ SUBDIR+=include/llvm-readobj
 SUBDIR+=llvm-readobj
 
 SUBDIR+=llvm-profdata
+SUBDIR+=llvm-cov
 
 .if ${AR_VERSION:L} == "llvm" || make(obj)
 SUBDIR+=libLLVMDlltoolDriver
Index: gnu/usr.bin/clang/llvm-cov/Makefile
===
RCS file: gnu/usr.bin/clang/llvm-cov/Makefile
diff -N gnu/usr.bin/clang/llvm-cov/Makefile
--- /dev/null   1 Jan 1970 00:00:00 -
+++ gnu/usr.bin/clang/llvm-cov/Makefile 29 Jul 2022 20:21:18 -
@@ -0,0 +1,29 @@
+# $OpenBSD$
+
+.include 
+
+PROG=  llvm-cov
+BINDIR=/usr/bin
+
+SRCS=  llvm-cov.cpp \
+   gcov.cpp \
+   CodeCoverage.cpp \
+   CoverageExporterJson.cpp \
+   CoverageExporterLcov.cpp \
+   CoverageFilters.cpp \
+   CoverageReport.cpp \
+   CoverageSummaryInfo.cpp \
+   SourceCoverageView.cpp \
+   SourceCoverageViewHTML.cpp \
+   SourceCoverageViewText.cpp \
+   TestingSupport.cpp \
+
+MAN+=  llvm-cov.1
+
+.PATH: ${.CURDIR}/../../../llvm/llvm/tools/llvm-cov
+
+LLVM_LIBDEPS=  LLVM
+
+LDADD+= -L ${.OBJDIR}/../libLLVM -lLLVM
+
+.include 
Index: gnu/usr.bin/clang/llvm-cov/llvm-cov.1
===
RCS file: gnu/usr.bin/clang/llvm-cov/llvm-cov.1
diff -N gnu/usr.bin/clang/llvm-cov/llvm-cov.1
--- /dev/null   1 Jan 1970 00:00:00 -
+++ gnu/usr.bin/clang/llvm-cov/llvm-cov.1   29 Jul 2022 20:21:18 -
@@ -0,0 +1,528 @@
+.\" Man page generated from reStructuredText.
+.
+.
+.nr rst2man-indent-level 0
+.
+.de1 rstReportMargin
+\\$1 \\n[an-margin]
+level \\n[rst2man-indent-level]
+level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
+-
+\\n[rst2man-indent0]
+\\n[rst2man-indent1]
+\\n[rst2man-indent2]
+..
+.de1 INDENT
+.\" .rstReportMargin pre:
+. RS \\$1
+. nr rst2man-indent\\n[rst2man-indent-level] \\n[an-margin]
+. nr rst2man-indent-level +1
+.\" .rstReportMargin post:
+..
+.de UNINDENT
+. RE
+.\" indent \\n[an-margin]
+.\" old: \\n[rst2man-indent\\n[rst2man-indent-level]]
+.nr rst2man-indent-level -1
+.\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
+.in \\n[rst2man-indent\\n[rst2man-indent-level]]u
+..
+.TH "LLVM-COV" "1" "2022-05-23" "13" "LLVM"
+.SH NAME
+llvm-cov \- emit coverage information
+.SH SYNOPSIS
+.sp
+\fBllvm\-cov\fP \fIcommand\fP [\fIargs...\fP]
+.SH DESCRIPTION
+.sp
+The \fBllvm\-cov\fP tool shows code coverage information for
+programs that are instrumented to emit profile data. It can be used to
+work with \fBgcov\fP\-style coverage or with \fBclang\fP\(aqs instrumentation
+based profiling.
+.sp
+If the program is invoked with a base name of \fBgcov\fP, it will behave as if
+the \fBllvm\-cov gcov\fP command were called. Otherwise, a command should
+be provided.
+.SH COMMANDS
+.INDENT 0.0
+.IP \(bu 2
+\fI\%gcov\fP
+.IP \(bu 2
+\fI\%show\fP
+.IP \(bu 2
+\fI\%report\fP
+.IP \(bu 2
+\fI\%export\fP
+.UNINDENT
+.SH GCOV COMMAND
+.SS SYNOPSIS
+.sp
+\fBllvm\-cov gcov\fP [\fIoptions\fP] \fISOURCEFILE\fP
+.SS DESCRIPTION
+.sp
+The \fBllvm\-cov gcov\fP tool reads code coverage data files and displays
+the coverage information for a specified source file. It is compatible with the
+\fBgcov\fP tool from version 4.2 of \fBGCC\fP and may also be compatible with 
some
+later versions of \fBgcov\fP\&.
+.sp
+To use \fBllvm\-cov gcov\fP, you must first build an instrumented version
+of your application that collects coverage data as it runs. Compile with the
+\fB\-fprofile\-arcs\fP and \fB\-ftest\-coverage\fP options to add the
+instrumentation. (Alternatively, you can use the \fB\-\-coverage\fP option, 
which
+includes both of those other options.)
+.sp
+At the time you compile the instrumented code, a \fB\&.gcno\fP data file will 
be
+generated for each object file. These \fB\&.gcno\fP files contain half of the
+coverage data. The other half of the data comes from \fB\&.gcda\fP files that 
are
+generated when you run the instrumented program, with a separate \fB\&.gcda\fP
+file for each object file. Each time you run the program, the execution counts
+are summed into any existing \fB\&.gcda\fP files, so be sure to remove any old
+files if you do not want their contents to be included.
+.sp
+By default, the \fB\&.gcda\fP files are written into the same directory as the
+object files, but you can override that by 

Re: Consistency and cleanup in /share/misc/airport

2022-07-29 Thread Thomas Wager
On Thu, 2022-07-28 at 21:45 -0400, Daniel Dickman wrote:
> 

> No thanks. These are pretty standard ways to refer to groups of
> airports that are close by. We have a bunch of these in the file and
> I'd hate to lose these.
> 
> BHZ:All Airports around Belo Horizonte, MG, Brazil
> BUE:All Airports around Buenos Aires, Argentina
> BUH:All Airports around Bucharest, Romania
> CHI:All Airports around Chicago, Illinois, USA
> DTT:All Airports around Detroit, Michigan, USA
> JKT:All Airports around Jakarta, Indonesia
> KCK:All Airports around Kansas City, Kansas, USA
> LON:All Airports around London, United Kingdom
> MFW:All Airports around Miami, Ft. Lauderdale and West Palm Beach,
> Florida, USA
> MIL:All Airports around Milano, Italy
> MOW:All Airports around Moscow, Russia
> NRW:All Airports around Nordrhein-Westfalen, Germany
> NYC:All Airports around New York City, New York, USA
> OSA:All Airports around Osaka, Japan
> OSL:All Airports around Oslo, Norway
> PAR:All Airports around Paris, France
> QDV:All Airports around Denver, Colorado, USA
> QLA:All Airports around Los Angeles, California, USA
> QSF:All Airports around San Francisco, California, USA
> RIO:All Airports around Rio de Janeiro, RJ, Brazil
> ROM:All Airports around Roma, Italy
> SAO:All Airports around Sao Paulo, SP, Brazil
> STO:All Airports around Stockholm, Sweden
> TYO:All Airports around Tokyo, Japan
> WAS:All Airports around Washington, District of Columbia, USA
> YEA:All Airports around Edmonton, Alberta, Canada
> YMQ:All Airports around Montreal, Quebec, Canada
> YTO:All Airports around Toronto, Ontario, Canad NSB:North Seaplane

I have seen those, and I am a bit confused. If these are supposed to be
IATA codes there are several problems:

KCK:
according to
https://en.wikipedia.org/wiki/Kirensk_Airport
and  
https://www.iata.org/en/publications/directories/code-search/?airport.search=KCK
this is Kirensk, Russia

MFW:
according to
https://en.wikipedia.org/wiki/List_of_airports_by_IATA_airport_code%3A_M
and
https://www.iata.org/en/publications/directories/code-search/?airport.search=MFW
this is Magaruque Island, Mozambique
although http://www.gcmap.com/airport/MFW says it is closed.

NRW:
according to
https://www.iata.org/en/publications/directories/code-search/?airport.search=NRW
this does not exist. But according to
https://iata.codes/airline/polizeifliegerstaffel-nordrhein-westfalen-nrw-hummel-de
there is 3-letter ICAO (not IATA) code NRW for "Polizeifliegerstaffel
Nordrhein-Westfalen", aka a police helicopter squad.

QDV:
according to
https://en.wikipedia.org/wiki/Jundia%C3%AD_Airport
and
https://www.iata.org/en/publications/directories/code-search/?airport.search=QDV
this is Comte. Rolim Adolfo Amaro, Jundiai, Brazil

QSF:
according to
https://en.wikipedia.org/wiki/Ain_Arnat_Airport
and
https://www.iata.org/en/publications/directories/code-search/?airport.search=QSF
this is Ain Arnat (also "8 May 1945"), Setif, Algeria

But if those are not IATA codes, then I am probably completely wrong
here.



Re: route(8), mention id(1)? [was Re: patch to ksh to show current rdomain]

2022-07-29 Thread Klemens Nanni
On Fri, Jul 29, 2022 at 03:29:01PM +0100, Stuart Henderson wrote:
> On 2022/07/29 14:01, Klemens Nanni wrote:
> > retrieving revision 1.103
> > diff -u -p -r1.103 route.8
> > --- sbin/route/route.8  31 Mar 2022 17:27:20 -  1.103
> > +++ sbin/route/route.8  29 Jul 2022 13:54:26 -
> > @@ -78,6 +78,9 @@ Suppress all output.
> >  .It Fl T Ar rtable
> >  Select an alternate routing table to modify or query.
> >  The default is to use the current routing table.
> > +.Pp
> > +The current routing table can be displayed with
> > +.Xr id 1 .
> 
> That's a good place for it. Think it would be better without .Pp,
> otherwise OK
> 
> >  .Dl # route -T4 exec /usr/sbin/sshd
> >  .Pp
> > -Display to which rdomain processes are assigned:
> > +Display to which routing table processes are assigned:
> 
> That's hard to parse. Maybe just "Display the routing table used by each
> process".

Committed with both, thanks.



Re: [v4] amd64: simplify TSC sync testing

2022-07-29 Thread Dave Voutila


Mark Kettenis  writes:

>> From: Dave Voutila 
>> Date: Fri, 29 Jul 2022 10:51:20 -0400
>>
>> Mark Kettenis  writes:
>>
>> >> From: Dave Voutila 
>> >> Date: Fri, 29 Jul 2022 10:10:01 -0400
>> >>
>> >> Scott Cheloha  writes:
>> >>
>> >> > On Thu, Jul 28, 2022 at 04:57:41PM -0400, Dave Voutila wrote:
>> >> >>
>> >> >> Stuart Henderson  writes:
>> >> >>
>> >> >> > On 2022/07/28 12:57, Scott Cheloha wrote:
>> >> >> >> On Thu, Jul 28, 2022 at 07:55:40AM -0400, Dave Voutila wrote:
>> >> >> >> >
>> >> >> >> > This is breaking timecounter selection on my x13 Ryzen 5 Pro 
>> >> >> >> > laptop
>> >> >> >> > running the latest kernel from snaps.
>> >> >> >>
>> >> >> >> Define "breaking".
>> >> >> >
>> >> >> > That's clear from the output:
>> >> >> >
>> >> >> > : On 2022/07/28 07:55, Dave Voutila wrote:
>> >> >> > : > $ sysctl -a | grep tsc
>> >> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
>> >> >> > : > acpitimer0(1000)
>> >> >> > : > machdep.tscfreq=2096064730
>> >> >> > : > machdep.invarianttsc=1
>> >> >> > : >
>> >> >> > : > $ sysctl kern.timecounter
>> >> >> > : > kern.timecounter.tick=1
>> >> >> > : > kern.timecounter.timestepwarnings=0
>> >> >> > : > kern.timecounter.hardware=i8254
>> >> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
>> >> >> > : > acpitimer0(1000)
>> >> >> >
>> >> >> >> The code detects TSC desync and marks the timecounter non-monotonic.
>> >> >> >
>> >> >> > That's good (and I think as would have happened before)
>> >> >> >
>> >> >> >> So it uses the i8254 instead.
>> >> >> >
>> >> >> > But that's not so good, there are higher prio timecounters available,
>> >> >> > acpihpet0 and acpitimer0, which would be better choices than i8254.
>> >> >>
>> >> >> Exactly my point. Thanks Stuart.
>> >> >
>> >> > Okay, please try this patch on the machine in question.
>> >>
>> >> That fixes the selection on my x13 gen1; it's choosing acpihpet0 now. No
>> >> issue with suspend/resume cycles either.
>> >>
>> >> Also tested the patch on my dual-socket Xeon machine and it looks to
>> >> still be properly synchronizing and selecting tsc as with the previous
>> >> diff & snapshot kernel.
>> >>
>> >> Is there any special consideration for unhiberate? I can't tell if/when
>> >> it is checking the TSCs across the cpus.
>> >
>> > Based on the link Scott posted yesterday, it would be interesting to
>> > see if there is a difference between a cold boot and a warm boot.
>> > Does it pick the TSC after a cold boot?  And if so, what happens if
>> > you hibernate after a warm boot (with the HPET as source) and
>> > unhibernate after a cold boot.
>> >
>>
>> Hmm...what's the best way to force cold/warm on an x13 Ryzen system? Do
>> I need to do this from UEFI?
>
> With cold boot I mean pressing the powerbutton after brining the
> machine down with shutdown -hp.  Warm boot is simply doing a reboot.

Ah, ok. I see no difference in the TSC sync results or the selection of
the timecounter (selects acpihpet0).

-dv



Re: [v4] amd64: simplify TSC sync testing

2022-07-29 Thread Mark Kettenis
> From: Dave Voutila 
> Date: Fri, 29 Jul 2022 10:51:20 -0400
> 
> Mark Kettenis  writes:
> 
> >> From: Dave Voutila 
> >> Date: Fri, 29 Jul 2022 10:10:01 -0400
> >>
> >> Scott Cheloha  writes:
> >>
> >> > On Thu, Jul 28, 2022 at 04:57:41PM -0400, Dave Voutila wrote:
> >> >>
> >> >> Stuart Henderson  writes:
> >> >>
> >> >> > On 2022/07/28 12:57, Scott Cheloha wrote:
> >> >> >> On Thu, Jul 28, 2022 at 07:55:40AM -0400, Dave Voutila wrote:
> >> >> >> >
> >> >> >> > This is breaking timecounter selection on my x13 Ryzen 5 Pro laptop
> >> >> >> > running the latest kernel from snaps.
> >> >> >>
> >> >> >> Define "breaking".
> >> >> >
> >> >> > That's clear from the output:
> >> >> >
> >> >> > : On 2022/07/28 07:55, Dave Voutila wrote:
> >> >> > : > $ sysctl -a | grep tsc
> >> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
> >> >> > : > acpitimer0(1000)
> >> >> > : > machdep.tscfreq=2096064730
> >> >> > : > machdep.invarianttsc=1
> >> >> > : >
> >> >> > : > $ sysctl kern.timecounter
> >> >> > : > kern.timecounter.tick=1
> >> >> > : > kern.timecounter.timestepwarnings=0
> >> >> > : > kern.timecounter.hardware=i8254
> >> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
> >> >> > : > acpitimer0(1000)
> >> >> >
> >> >> >> The code detects TSC desync and marks the timecounter non-monotonic.
> >> >> >
> >> >> > That's good (and I think as would have happened before)
> >> >> >
> >> >> >> So it uses the i8254 instead.
> >> >> >
> >> >> > But that's not so good, there are higher prio timecounters available,
> >> >> > acpihpet0 and acpitimer0, which would be better choices than i8254.
> >> >>
> >> >> Exactly my point. Thanks Stuart.
> >> >
> >> > Okay, please try this patch on the machine in question.
> >>
> >> That fixes the selection on my x13 gen1; it's choosing acpihpet0 now. No
> >> issue with suspend/resume cycles either.
> >>
> >> Also tested the patch on my dual-socket Xeon machine and it looks to
> >> still be properly synchronizing and selecting tsc as with the previous
> >> diff & snapshot kernel.
> >>
> >> Is there any special consideration for unhiberate? I can't tell if/when
> >> it is checking the TSCs across the cpus.
> >
> > Based on the link Scott posted yesterday, it would be interesting to
> > see if there is a difference between a cold boot and a warm boot.
> > Does it pick the TSC after a cold boot?  And if so, what happens if
> > you hibernate after a warm boot (with the HPET as source) and
> > unhibernate after a cold boot.
> >
> 
> Hmm...what's the best way to force cold/warm on an x13 Ryzen system? Do
> I need to do this from UEFI?

With cold boot I mean pressing the powerbutton after brining the
machine down with shutdown -hp.  Warm boot is simply doing a reboot.



Weakness of Internet of Things security

2022-07-29 Thread Nelson H. F. Beebe
Recent traffic on this list is addressing OpenBSD issues with respect
to randomness for improving security.

There is an excellent survey published today of problems with Internet
of Things security, which in turn affects O/S development, written by
two prominent cryptographers, one of whom is the co-inventor of public
key cryptography:

James P. Hughes and Whitfield Diffie
The Challenges of IoT, TLS, and Random Number Generators in
the Real World: Bad random numbers are still with us and are
proliferating in modern systems
Queue 20(3) 18--40 May 2022
https://doi.org/10.1145/3546933

One of its 20 references is a 2016 paper that reports Internet
measurements on the extent of key weakness:

Weak Keys Remain Widespread in Network Devices
https://doi.org/10.1145/2987443.2987486

---
- Nelson H. F. BeebeTel: +1 801 581 5254  -
- University of Utah  -
- Department of Mathematics, 110 LCBInternet e-mail: be...@math.utah.edu  -
- 155 S 1400 E RM 233   be...@acm.org  be...@computer.org -
- Salt Lake City, UT 84112-0090, USAURL: http://www.math.utah.edu/~beebe/ -
---



Re: randomise arc4random() rekey interval

2022-07-29 Thread Theo de Raadt
>Another option is to move the _rs_stir_if_needed() calls from
>_rs_random_u32() and _rs_random_buf() to arc4random() and
>arc4random_buf(). The latter two are the subsystem's entry points.

That requires more careful review of the -portable versions in libcrypto.
It seems they were forgotten.  Maybe in a few years some of these will go
away..

>Taking the fuzz value directly from getentropy() would be a clear
>approach that does not add odd hoops, though some might feel it
>uneconomic use of system entropy. ;)

There is no such thing as 'system entropy' cost.  The kernel buffer is mostly
a long-duration chacha also, with the occasional refresh to pull from another
sequence which does not measure entropy at all.  This measuring entropy idea is
garbage, and dead.  The only concern for not doing this is system call overhead.

arc4random one of the oldest ideas in computer science -- a local cache.




Re: [v4] amd64: simplify TSC sync testing

2022-07-29 Thread Dave Voutila


Mark Kettenis  writes:

>> From: Dave Voutila 
>> Date: Fri, 29 Jul 2022 10:10:01 -0400
>>
>> Scott Cheloha  writes:
>>
>> > On Thu, Jul 28, 2022 at 04:57:41PM -0400, Dave Voutila wrote:
>> >>
>> >> Stuart Henderson  writes:
>> >>
>> >> > On 2022/07/28 12:57, Scott Cheloha wrote:
>> >> >> On Thu, Jul 28, 2022 at 07:55:40AM -0400, Dave Voutila wrote:
>> >> >> >
>> >> >> > This is breaking timecounter selection on my x13 Ryzen 5 Pro laptop
>> >> >> > running the latest kernel from snaps.
>> >> >>
>> >> >> Define "breaking".
>> >> >
>> >> > That's clear from the output:
>> >> >
>> >> > : On 2022/07/28 07:55, Dave Voutila wrote:
>> >> > : > $ sysctl -a | grep tsc
>> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
>> >> > : > acpitimer0(1000)
>> >> > : > machdep.tscfreq=2096064730
>> >> > : > machdep.invarianttsc=1
>> >> > : >
>> >> > : > $ sysctl kern.timecounter
>> >> > : > kern.timecounter.tick=1
>> >> > : > kern.timecounter.timestepwarnings=0
>> >> > : > kern.timecounter.hardware=i8254
>> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
>> >> > : > acpitimer0(1000)
>> >> >
>> >> >> The code detects TSC desync and marks the timecounter non-monotonic.
>> >> >
>> >> > That's good (and I think as would have happened before)
>> >> >
>> >> >> So it uses the i8254 instead.
>> >> >
>> >> > But that's not so good, there are higher prio timecounters available,
>> >> > acpihpet0 and acpitimer0, which would be better choices than i8254.
>> >>
>> >> Exactly my point. Thanks Stuart.
>> >
>> > Okay, please try this patch on the machine in question.
>>
>> That fixes the selection on my x13 gen1; it's choosing acpihpet0 now. No
>> issue with suspend/resume cycles either.
>>
>> Also tested the patch on my dual-socket Xeon machine and it looks to
>> still be properly synchronizing and selecting tsc as with the previous
>> diff & snapshot kernel.
>>
>> Is there any special consideration for unhiberate? I can't tell if/when
>> it is checking the TSCs across the cpus.
>
> Based on the link Scott posted yesterday, it would be interesting to
> see if there is a difference between a cold boot and a warm boot.
> Does it pick the TSC after a cold boot?  And if so, what happens if
> you hibernate after a warm boot (with the HPET as source) and
> unhibernate after a cold boot.
>

Hmm...what's the best way to force cold/warm on an x13 Ryzen system? Do
I need to do this from UEFI?

-dv



Re: [v4] amd64: simplify TSC sync testing

2022-07-29 Thread Mark Kettenis
> From: Dave Voutila 
> Date: Fri, 29 Jul 2022 10:10:01 -0400
> 
> Scott Cheloha  writes:
> 
> > On Thu, Jul 28, 2022 at 04:57:41PM -0400, Dave Voutila wrote:
> >>
> >> Stuart Henderson  writes:
> >>
> >> > On 2022/07/28 12:57, Scott Cheloha wrote:
> >> >> On Thu, Jul 28, 2022 at 07:55:40AM -0400, Dave Voutila wrote:
> >> >> >
> >> >> > This is breaking timecounter selection on my x13 Ryzen 5 Pro laptop
> >> >> > running the latest kernel from snaps.
> >> >>
> >> >> Define "breaking".
> >> >
> >> > That's clear from the output:
> >> >
> >> > : On 2022/07/28 07:55, Dave Voutila wrote:
> >> > : > $ sysctl -a | grep tsc
> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
> >> > : > acpitimer0(1000)
> >> > : > machdep.tscfreq=2096064730
> >> > : > machdep.invarianttsc=1
> >> > : >
> >> > : > $ sysctl kern.timecounter
> >> > : > kern.timecounter.tick=1
> >> > : > kern.timecounter.timestepwarnings=0
> >> > : > kern.timecounter.hardware=i8254
> >> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
> >> > : > acpitimer0(1000)
> >> >
> >> >> The code detects TSC desync and marks the timecounter non-monotonic.
> >> >
> >> > That's good (and I think as would have happened before)
> >> >
> >> >> So it uses the i8254 instead.
> >> >
> >> > But that's not so good, there are higher prio timecounters available,
> >> > acpihpet0 and acpitimer0, which would be better choices than i8254.
> >>
> >> Exactly my point. Thanks Stuart.
> >
> > Okay, please try this patch on the machine in question.
> 
> That fixes the selection on my x13 gen1; it's choosing acpihpet0 now. No
> issue with suspend/resume cycles either.
> 
> Also tested the patch on my dual-socket Xeon machine and it looks to
> still be properly synchronizing and selecting tsc as with the previous
> diff & snapshot kernel.
> 
> Is there any special consideration for unhiberate? I can't tell if/when
> it is checking the TSCs across the cpus.

Based on the link Scott posted yesterday, it would be interesting to
see if there is a difference between a cold boot and a warm boot.
Does it pick the TSC after a cold boot?  And if so, what happens if
you hibernate after a warm boot (with the HPET as source) and
unhibernate after a cold boot.

> > It adds a tc_detach() function to kern_tc.c.  The first time we fail
> > the sync test, the BP calls tc_detach(), changes the TSC's tc_quality
> > to a negative value to tell everyone "this is not monotonic", then
> > reinstalls the TSC timecounter again with tc_init().
> >
> > Because we are making this call *once*, from one place, I do not think
> > the O(n) removal time matters, so I have not switched the tc_list from
> > SLIST to TAILQ.
> >
> > It is possible for a thread to be asleep in sysctl_tc_hardware()
> > during resume, but the thread would be done iterating through the list
> > if it had reached rw_enter_write(), so removing/adding tsc_timecounter
> > to the list during resume cannot break list traversal.
> >
> > Switching the active timecounter during resume is also fine.  The only
> > race is with tc_adjfreq().  If a thread is asleep in adjfreq(2) when
> > the system suspends, and we change the active timecounter during
> > resume, the frequency change may be applied to the "wrong" timecounter.
> >
> > ... but this is always a race, because adjfreq(2) only operates on the
> > active timecounter, and root can change it at any time via sysctl(2).
> > So it's not a new problem.
> >
> > ...
> >
> > It might be simpler to just change tc_lock from a rwlock to a mutex.
> > Then the MP analysis is much simpler across a suspend/resume.
> >
> > Index: sys/arch/amd64/amd64/tsc.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> > retrieving revision 1.24
> > diff -u -p -r1.24 tsc.c
> > --- sys/arch/amd64/amd64/tsc.c  31 Aug 2021 15:11:54 -  1.24
> > +++ sys/arch/amd64/amd64/tsc.c  29 Jul 2022 01:06:17 -
> > @@ -36,13 +36,6 @@ int  tsc_recalibrate;
> >  uint64_t   tsc_frequency;
> >  inttsc_is_invariant;
> >
> > -#defineTSC_DRIFT_MAX   250
> > -#define TSC_SKEW_MAX   100
> > -int64_ttsc_drift_observed;
> > -
> > -volatile int64_t   tsc_sync_val;
> > -volatile struct cpu_info   *tsc_sync_cpu;
> > -
> >  u_int  tsc_get_timecount(struct timecounter *tc);
> >  void   tsc_delay(int usecs);
> >
> > @@ -236,22 +229,12 @@ cpu_recalibrate_tsc(struct timecounter *
> >  u_int
> >  tsc_get_timecount(struct timecounter *tc)
> >  {
> > -   return rdtsc_lfence() + curcpu()->ci_tsc_skew;
> > +   return rdtsc_lfence();
> >  }
> >
> >  void
> >  tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
> >  {
> > -#ifdef TSC_DEBUG
> > -   printf("%s: TSC skew=%lld observed drift=%lld\n", ci->ci_dev->dv_xname,
> > -   (long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
> > -#endif
> > -   if (ci->ci_tsc_skew < 

Re: [v4] amd64: simplify TSC sync testing

2022-07-29 Thread Dave Voutila


Scott Cheloha  writes:

> On Thu, Jul 28, 2022 at 04:57:41PM -0400, Dave Voutila wrote:
>>
>> Stuart Henderson  writes:
>>
>> > On 2022/07/28 12:57, Scott Cheloha wrote:
>> >> On Thu, Jul 28, 2022 at 07:55:40AM -0400, Dave Voutila wrote:
>> >> >
>> >> > This is breaking timecounter selection on my x13 Ryzen 5 Pro laptop
>> >> > running the latest kernel from snaps.
>> >>
>> >> Define "breaking".
>> >
>> > That's clear from the output:
>> >
>> > : On 2022/07/28 07:55, Dave Voutila wrote:
>> > : > $ sysctl -a | grep tsc
>> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
>> > : > acpitimer0(1000)
>> > : > machdep.tscfreq=2096064730
>> > : > machdep.invarianttsc=1
>> > : >
>> > : > $ sysctl kern.timecounter
>> > : > kern.timecounter.tick=1
>> > : > kern.timecounter.timestepwarnings=0
>> > : > kern.timecounter.hardware=i8254
>> > : > kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000)
>> > : > acpitimer0(1000)
>> >
>> >> The code detects TSC desync and marks the timecounter non-monotonic.
>> >
>> > That's good (and I think as would have happened before)
>> >
>> >> So it uses the i8254 instead.
>> >
>> > But that's not so good, there are higher prio timecounters available,
>> > acpihpet0 and acpitimer0, which would be better choices than i8254.
>>
>> Exactly my point. Thanks Stuart.
>
> Okay, please try this patch on the machine in question.

That fixes the selection on my x13 gen1; it's choosing acpihpet0 now. No
issue with suspend/resume cycles either.

Also tested the patch on my dual-socket Xeon machine and it looks to
still be properly synchronizing and selecting tsc as with the previous
diff & snapshot kernel.

Is there any special consideration for unhiberate? I can't tell if/when
it is checking the TSCs across the cpus.

>
> It adds a tc_detach() function to kern_tc.c.  The first time we fail
> the sync test, the BP calls tc_detach(), changes the TSC's tc_quality
> to a negative value to tell everyone "this is not monotonic", then
> reinstalls the TSC timecounter again with tc_init().
>
> Because we are making this call *once*, from one place, I do not think
> the O(n) removal time matters, so I have not switched the tc_list from
> SLIST to TAILQ.
>
> It is possible for a thread to be asleep in sysctl_tc_hardware()
> during resume, but the thread would be done iterating through the list
> if it had reached rw_enter_write(), so removing/adding tsc_timecounter
> to the list during resume cannot break list traversal.
>
> Switching the active timecounter during resume is also fine.  The only
> race is with tc_adjfreq().  If a thread is asleep in adjfreq(2) when
> the system suspends, and we change the active timecounter during
> resume, the frequency change may be applied to the "wrong" timecounter.
>
> ... but this is always a race, because adjfreq(2) only operates on the
> active timecounter, and root can change it at any time via sysctl(2).
> So it's not a new problem.
>
> ...
>
> It might be simpler to just change tc_lock from a rwlock to a mutex.
> Then the MP analysis is much simpler across a suspend/resume.
>
> Index: sys/arch/amd64/amd64/tsc.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 tsc.c
> --- sys/arch/amd64/amd64/tsc.c31 Aug 2021 15:11:54 -  1.24
> +++ sys/arch/amd64/amd64/tsc.c29 Jul 2022 01:06:17 -
> @@ -36,13 +36,6 @@ inttsc_recalibrate;
>  uint64_t tsc_frequency;
>  int  tsc_is_invariant;
>
> -#define  TSC_DRIFT_MAX   250
> -#define TSC_SKEW_MAX 100
> -int64_t  tsc_drift_observed;
> -
> -volatile int64_t tsc_sync_val;
> -volatile struct cpu_info *tsc_sync_cpu;
> -
>  u_inttsc_get_timecount(struct timecounter *tc);
>  void tsc_delay(int usecs);
>
> @@ -236,22 +229,12 @@ cpu_recalibrate_tsc(struct timecounter *
>  u_int
>  tsc_get_timecount(struct timecounter *tc)
>  {
> - return rdtsc_lfence() + curcpu()->ci_tsc_skew;
> + return rdtsc_lfence();
>  }
>
>  void
>  tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
>  {
> -#ifdef TSC_DEBUG
> - printf("%s: TSC skew=%lld observed drift=%lld\n", ci->ci_dev->dv_xname,
> - (long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
> -#endif
> - if (ci->ci_tsc_skew < -TSC_SKEW_MAX || ci->ci_tsc_skew > TSC_SKEW_MAX) {
> - printf("%s: disabling user TSC (skew=%lld)\n",
> - ci->ci_dev->dv_xname, (long long)ci->ci_tsc_skew);
> - tsc_timecounter.tc_user = 0;
> - }
> -
>   if (!(ci->ci_flags & CPUF_PRIMARY) ||
>   !(ci->ci_flags & CPUF_CONST_TSC) ||
>   !(ci->ci_flags & CPUF_INVAR_TSC))
> @@ -268,111 +251,276 @@ tsc_timecounter_init(struct cpu_info *ci
>   calibrate_tsc_freq();
>   }
>
> - if (tsc_drift_observed > TSC_DRIFT_MAX) {
> - printf("ERROR: %lld 

Re: route(8), mention id(1)? [was Re: patch to ksh to show current rdomain]

2022-07-29 Thread Stuart Henderson
On 2022/07/29 14:01, Klemens Nanni wrote:
> retrieving revision 1.103
> diff -u -p -r1.103 route.8
> --- sbin/route/route.831 Mar 2022 17:27:20 -  1.103
> +++ sbin/route/route.829 Jul 2022 13:54:26 -
> @@ -78,6 +78,9 @@ Suppress all output.
>  .It Fl T Ar rtable
>  Select an alternate routing table to modify or query.
>  The default is to use the current routing table.
> +.Pp
> +The current routing table can be displayed with
> +.Xr id 1 .

That's a good place for it. Think it would be better without .Pp,
otherwise OK

>  .Dl # route -T4 exec /usr/sbin/sshd
>  .Pp
> -Display to which rdomain processes are assigned:
> +Display to which routing table processes are assigned:

That's hard to parse. Maybe just "Display the routing table used by each
process".

>  .Pp
>  .Dl $ ps aux -o rtable
>  .Pp
> +Display the routing table of the current process:
> +.Pp
> +.Dl $ id -R
> +.Pp
>  A
>  .Xr pf.conf 5
>  snippet to block incoming port 80,
> @@ -133,6 +137,7 @@ Delete rdomain 4 again:
>  # ifconfig lo4 destroy
>  .Ed
>  .Sh SEE ALSO
> +.Xr id 1 ,
>  .Xr netstat 1 ,
>  .Xr ps 1 ,
>  .Xr lo 4 ,
> 



Re: route(8), mention id(1)? [was Re: patch to ksh to show current rdomain]

2022-07-29 Thread Klemens Nanni
On Tue, Jul 26, 2022 at 12:09:29PM +, Klemens Nanni wrote:
> On Tue, Jul 26, 2022 at 12:44:20PM +0100, Stuart Henderson wrote:
> > I tried looking for a command to do this and completely failed
> > hence the ugly ps command. Do we want something like this?
> 
> I think it's useful.  There's also netstat(1)' -R but that just dumps
> everything without indication where the current process is.
> 
> > Index: route.8
> > ===
> > RCS file: /cvs/src/sbin/route/route.8,v
> > retrieving revision 1.103
> > diff -u -p -r1.103 route.8
> > --- route.8 31 Mar 2022 17:27:20 -  1.103
> > +++ route.8 26 Jul 2022 11:41:41 -
> > @@ -102,6 +102,8 @@ Execute a command forcing the process an
> >  routing table and appropriate routing domain as specified with the
> >  .Fl T Ar rtable
> >  option.
> > +The current routing table can be displayed with
> > +.Xr id 1 .
> 
> Perhaps put this under the -T rtable description?  There you'll spot it
> earlier and every route command suports [-T rtable].
> 
> >  .It Xo
> >  .Nm route
> >  .Op Fl nqv
> > @@ -625,6 +627,7 @@ to create the new entry.
> >  .El
> >  .Sh SEE ALSO
> >  .Xr netstat 1 ,
> > +.Xr getrtable 2 ,
> 
> I don't think this is warranted.
> 
> >  .Xr gethostbyname 3 ,
> >  .Xr netintro 4 ,
> >  .Xr route 4 ,
> 
> rtable(4) has a bunch of examples but also lacks id -R and is not
> referenced where one might find it useful:
> 
>   $ man -k xr~'^(rdomain|rtable)'
>   pair(4) - virtual Ethernet interface pair
>   route(4) - kernel packet forwarding database
>   wg(4) - WireGuard pseudo-device
>   bgpd.conf(5) - Border Gateway Protocol daemon configuration file
>   sshd_config(5) - OpenSSH daemon configuration file
>   ifconfig(8) - configure network interface parameters
>   route, rt_setgate, rtdeletemsg, rtredirect(9) - kernel routing interface
>   rtalloc, rtalloc_mpath, rtfree, rtisvalid, rtref(9) - routing entry 
> interface
> 
> So instead maybe point at rtable(4) from here (and show id -R there)?

Like this.

Feedback? Objection? OK?


Index: sbin/route/route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.103
diff -u -p -r1.103 route.8
--- sbin/route/route.8  31 Mar 2022 17:27:20 -  1.103
+++ sbin/route/route.8  29 Jul 2022 13:54:26 -
@@ -78,6 +78,9 @@ Suppress all output.
 .It Fl T Ar rtable
 Select an alternate routing table to modify or query.
 The default is to use the current routing table.
+.Pp
+The current routing table can be displayed with
+.Xr id 1 .
 .It Fl t
 Write routing messages to a fake device
 .Pa ( /dev/null )
@@ -624,10 +627,12 @@ low on resources and was unable to alloc
 to create the new entry.
 .El
 .Sh SEE ALSO
+.Xr id 1 ,
 .Xr netstat 1 ,
 .Xr gethostbyname 3 ,
 .Xr netintro 4 ,
 .Xr route 4 ,
+.Xr rtable 4 ,
 .Xr tcp 4 ,
 .Xr hosts 5 ,
 .Xr mygate 5 ,
Index: share/man/man4/rdomain.4
===
RCS file: /cvs/src/share/man/man4/rdomain.4,v
retrieving revision 1.18
diff -u -p -r1.18 rdomain.4
--- share/man/man4/rdomain.431 Mar 2022 17:27:21 -  1.18
+++ share/man/man4/rdomain.429 Jul 2022 14:00:03 -
@@ -114,10 +114,14 @@ in rtable 4:
 .Pp
 .Dl # route -T4 exec /usr/sbin/sshd
 .Pp
-Display to which rdomain processes are assigned:
+Display to which routing table processes are assigned:
 .Pp
 .Dl $ ps aux -o rtable
 .Pp
+Display the routing table of the current process:
+.Pp
+.Dl $ id -R
+.Pp
 A
 .Xr pf.conf 5
 snippet to block incoming port 80,
@@ -133,6 +137,7 @@ Delete rdomain 4 again:
 # ifconfig lo4 destroy
 .Ed
 .Sh SEE ALSO
+.Xr id 1 ,
 .Xr netstat 1 ,
 .Xr ps 1 ,
 .Xr lo 4 ,



Re: randomise arc4random() rekey interval

2022-07-29 Thread Visa Hankala
On Fri, Jul 29, 2022 at 06:56:08AM -0600, Theo de Raadt wrote:
> Visa Hankala  wrote:
> 
> > On Thu, Jul 28, 2022 at 11:00:12AM +1000, Damien Miller wrote:
> > > + rs->rs_count = REKEY_BASE;
> > > + /* rekey interval should not be predictable */
> > > + _rs_random_u32(_fuzz);
> > > + rs->rs_count += rekey_fuzz % REKEY_BASE;
> > 
> > The randomization looks good.
> > 
> > However, might it cause a problem (in the future) that _rs_random_u32()
> > calls _rs_stir_if_needed()? rs_count has a largish value so a recursive
> > invocation of _rs_stir() should not happen, but anyway.
> 
> The question is what _rs_random_u32() will do when it calls 
> _rs_stir_if_needed().
> 
> There is one potential problem.  lib/libcrypto/arc4random/*.h contains 
> portable
> wrappers for _rs_forkdetect(), which actually do things.  memset(rs, 0, 
> sizeof(*rs))
> will trash the rs state. Let's imagine a "fork" has happened same time that 
> bytes
> run out.
> 
> _rs_stir()
> ...
> rs->rs_count = REKEY_BASE;
> _rs_random_u32 -> _rs_stir_if_needed -> _rs_forkdetect
>   - all rs fields are zero'd with memset
>   - _rs_forkdetect returns
> 
> back in _rs_stir_if_needed,
>- if (!rs || rs->rs_count <= len)
>_rs_stir();
> 
> 
> So it will recurse once (only once, because a 2nd fork cannot happen).
> But this is fragile.
> 
> Alternatives are to get the value direct from getentropy -- with KEYSZ + IVSZ 
> + 4
> maybe?  Or fetch a value for this random bias early and track it in rs, but 
> don't
> damage it in the memset?  Or split _rs_random_u32() so that a sub-function of 
> it
> may collect these 4 keystream bytes without the _rs_stir_if_needed/_rs_rekey 
> checks?

_rs_stir() clears rs_buf, so a rekey is needed if the fuzz value is
taken from the keystream.

Another option is to move the _rs_stir_if_needed() calls from
_rs_random_u32() and _rs_random_buf() to arc4random() and
arc4random_buf(). The latter two are the subsystem's entry points.

Taking the fuzz value directly from getentropy() would be a clear
approach that does not add odd hoops, though some might feel it
uneconomic use of system entropy. ;)



Re: ts(1): parse input format string only once

2022-07-29 Thread Scott Cheloha
On Wed, Jul 13, 2022 at 12:50:24AM -0500, Scott Cheloha wrote:
> We reduce overhead if we only parse the user's format string once.  To
> achieve that, this patch does the following:
> 
> [...]
> 
> - When parsing the user format string in fmtfmt(), keep a list of
>   where each microsecond substring lands in buf.  We'll need it later.
> 
> - Move the printing part of fmtfmt() into a new function, fmtprint().
>   fmtprint() is now called from the main loop instead of fmtfmt().
> 
> - In fmtprint(), before calling strftime(3), update any microsecond
>   substrings in buf using the list we built earlier in fmtfmt().  Note
>   that if there aren't any such substrings we don't call snprintf(3)
>   at all.
> 
> [...]

Two week bump.

Here is a stripped-down patch with only the above changes.  Hopefully
this makes the intent of the patch more obvious.

In short, parse the user format string only once, and then only update
the microsecond parts (if any) when we print each new timestamp.

Index: ts.c
===
RCS file: /cvs/src/usr.bin/ts/ts.c,v
retrieving revision 1.8
diff -u -p -r1.8 ts.c
--- ts.c7 Jul 2022 10:40:25 -   1.8
+++ ts.c29 Jul 2022 13:12:07 -
@@ -17,6 +17,7 @@
  */
 
 #include 
+#include 
 #include 
 
 #include 
@@ -27,13 +28,20 @@
 #include 
 #include 
 
+SIMPLEQ_HEAD(, usec) usec_queue = SIMPLEQ_HEAD_INITIALIZER(usec_queue);
+struct usec {
+   SIMPLEQ_ENTRY(usec) next;
+   char *pos;
+};
+
 static char*format = "%b %d %H:%M:%S";
 static char*buf;
 static char*outbuf;
 static size_t   bufsize;
 static size_t   obsize;
 
-static void fmtfmt(const struct timespec *);
+static void fmtfmt(void);
+static void fmtprint(const struct timespec *);
 static void __dead  usage(void);
 
 int
@@ -88,6 +96,8 @@ main(int argc, char *argv[])
if ((outbuf = calloc(1, obsize)) == NULL)
err(1, NULL);
 
+   fmtfmt();
+
/* force UTC for interval calculations */
if (iflag || sflag)
if (setenv("TZ", "UTC", 1) == -1)
@@ -106,7 +116,7 @@ main(int argc, char *argv[])
timespecadd(, _offset, );
else
ts = now;
-   fmtfmt();
+   fmtprint();
if (iflag)
start = now;
}
@@ -132,15 +142,11 @@ usage(void)
  * so you can format while you format
  */
 static void
-fmtfmt(const struct timespec *ts)
+fmtfmt(void)
 {
-   struct tm *tm;
-   char *f, us[7];
-
-   if ((tm = localtime(>tv_sec)) == NULL)
-   err(1, "localtime");
+   char *f;
+   struct usec *u;
 
-   snprintf(us, sizeof(us), "%06ld", ts->tv_nsec / 1000);
strlcpy(buf, format, bufsize);
f = buf;
 
@@ -159,12 +165,34 @@ fmtfmt(const struct timespec *ts)
f[0] = f[1];
f[1] = '.';
f += 2;
+   u = malloc(sizeof u);
+   if (u == NULL)
+   err(1, NULL);
+   u->pos = f;
+   SIMPLEQ_INSERT_TAIL(_queue, u, next);
l = strlen(f);
memmove(f + 6, f, l + 1);
-   memcpy(f, us, 6);
f += 6;
}
} while (*f != '\0');
+}
+
+static void
+fmtprint(const struct timespec *ts)
+{
+   char us[8];
+   struct tm *tm;
+   struct usec *u;
+
+   if ((tm = localtime(>tv_sec)) == NULL)
+   err(1, "localtime");
+
+   /* Update any microsecond substrings in the format buffer. */
+   if (!SIMPLEQ_EMPTY(_queue)) {
+   snprintf(us, sizeof(us), "%06ld", ts->tv_nsec / 1000);
+   SIMPLEQ_FOREACH(u, _queue, next)
+   memcpy(u->pos, us, 6);
+   }
 
*outbuf = '\0';
if (*buf != '\0') {



Re: randomise arc4random() rekey interval

2022-07-29 Thread Theo de Raadt
Visa Hankala  wrote:

> On Thu, Jul 28, 2022 at 11:00:12AM +1000, Damien Miller wrote:
> > +   rs->rs_count = REKEY_BASE;
> > +   /* rekey interval should not be predictable */
> > +   _rs_random_u32(_fuzz);
> > +   rs->rs_count += rekey_fuzz % REKEY_BASE;
> 
> The randomization looks good.
> 
> However, might it cause a problem (in the future) that _rs_random_u32()
> calls _rs_stir_if_needed()? rs_count has a largish value so a recursive
> invocation of _rs_stir() should not happen, but anyway.

The question is what _rs_random_u32() will do when it calls 
_rs_stir_if_needed().

There is one potential problem.  lib/libcrypto/arc4random/*.h contains portable
wrappers for _rs_forkdetect(), which actually do things.  memset(rs, 0, 
sizeof(*rs))
will trash the rs state. Let's imagine a "fork" has happened same time that 
bytes
run out.

_rs_stir()
...
rs->rs_count = REKEY_BASE;
_rs_random_u32 -> _rs_stir_if_needed -> _rs_forkdetect
  - all rs fields are zero'd with memset
  - _rs_forkdetect returns

back in _rs_stir_if_needed,
 - if (!rs || rs->rs_count <= len)
   _rs_stir();


So it will recurse once (only once, because a 2nd fork cannot happen).
But this is fragile.

Alternatives are to get the value direct from getentropy -- with KEYSZ + IVSZ + 
4
maybe?  Or fetch a value for this random bias early and track it in rs, but 
don't
damage it in the memset?  Or split _rs_random_u32() so that a sub-function of it
may collect these 4 keystream bytes without the _rs_stir_if_needed/_rs_rekey 
checks?



Re: randomise arc4random() rekey interval

2022-07-29 Thread Visa Hankala
On Thu, Jul 28, 2022 at 11:00:12AM +1000, Damien Miller wrote:
> + rs->rs_count = REKEY_BASE;
> + /* rekey interval should not be predictable */
> + _rs_random_u32(_fuzz);
> + rs->rs_count += rekey_fuzz % REKEY_BASE;

The randomization looks good.

However, might it cause a problem (in the future) that _rs_random_u32()
calls _rs_stir_if_needed()? rs_count has a largish value so a recursive
invocation of _rs_stir() should not happen, but anyway.



Re: remove pre-486 code from i386 platform

2022-07-29 Thread Daniel Dickman



> On Jul 29, 2022, at 2:18 AM, Tomasz Rola  wrote:
> 
> On Thu, Jul 28, 2022 at 08:06:28PM -0400, Daniel Dickman wrote:
>>> On Thu, Jul 28, 2022 at 3:37 AM Jeroen Massar  wrote:
>>> 
> [...]
>>> 
>>> I personally would not touch the .h definitions:
>>> 
 
 Index: i386/include/cputypes.h
 [..]
 
 -#define  CPUCLASS_3860
 #define   CPUCLASS_4861
> [...]
>>> Though, one could argue for the include users that they should also be 
>>> stripping their code of this use and super special casing...
>> 
>> How much of a real problem is this? These defines are ONLY available
>> on the i386 platform.
>> 
>> Wouldn't you have to be doing something super unportable to begin with
>> if you're using these defines for anything?
> 
> OTOH, if someone would want to do something super unportable, would
> you tell them to use some other operating system

Shrug. People can use whatever systems they like. I certainly do. You can too. 
This isn’t really a technical argument. It’s just an irritating comment.

> I guess it is ok to drop supporting code but...

It’s not a matter of dropping support. The current code already can’t possibly 
work on a 386. Support was dropped long ago.

> 
> I think it would be prudent to keep those few old defines, just mark
> them with relevant comment saying they are no longer being used and
> only kept to prevent someone clever from redefining those values for
> whatever clever purpose they will see in a future.

> I believe it is still possible to buy 386 in a form of so called
> industrial computer.

As I pointed out in my original email, the current code can’t possibly run on a 
real 386.

> Their life is expected to be decades
> long.
> Someone out there might be supporting this old stuff while
> making sure his
> code compiles on newer hardware, too. While not having
> the defines in *.h should be ok, it does not really cost much to keep
> them.

Should we add defines for the 286 too?

> 
> -- 
> Regards,
> Tomasz Rola
> 
> --
> ** A C programmer asked whether computer had Buddha's nature.  **
> ** As the answer, master did "rm -rif" on the programmer's home**
> ** directory. And then the C programmer became enlightened...  **
> ** **
> ** Tomasz Rola  mailto:tomasz_r...@bigfoot.com **
> 



Re: remove pre-486 code from i386 platform

2022-07-29 Thread Jonathan Gray
On Fri, Jul 29, 2022 at 06:58:08AM +0200, Tomasz Rola wrote:
> On Thu, Jul 28, 2022 at 08:06:28PM -0400, Daniel Dickman wrote:
> > On Thu, Jul 28, 2022 at 3:37 AM Jeroen Massar  wrote:
> > >
> [...]
> > >
> > > I personally would not touch the .h definitions:
> > >
> > > >
> > > > Index: i386/include/cputypes.h
> > > > [..]
> > > >
> > > > -#define  CPUCLASS_3860
> > > > #define   CPUCLASS_4861
> [...]
> > > Though, one could argue for the include users that they should also be 
> > > stripping their code of this use and super special casing...
> > 
> > How much of a real problem is this? These defines are ONLY available
> > on the i386 platform.
> > 
> > Wouldn't you have to be doing something super unportable to begin with
> > if you're using these defines for anything?
> 
> OTOH, if someone would want to do something super unportable, would
> you tell them to use some other operating system?
> 
> I guess it is ok to drop supporting code but...
> 
> I think it would be prudent to keep those few old defines, just mark
> them with relevant comment saying they are no longer being used and
> only kept to prevent someone clever from redefining those values for
> whatever clever purpose they will see in a future.

the numbers are not significant

> 
> I believe it is still possible to buy 386 in a form of so called
> industrial computer. Their life is expected to be decades
> long. Someone out there might be supporting this old stuff while
> making sure his code compiles on newer hardware, too. While not having
> the defines in *.h should be ok, it does not really cost much to keep
> them.

386 isn't supported.  This patch is some minor cleanup and doesn't
change that.



Re: remove pre-486 code from i386 platform

2022-07-29 Thread Tomasz Rola
On Thu, Jul 28, 2022 at 08:06:28PM -0400, Daniel Dickman wrote:
> On Thu, Jul 28, 2022 at 3:37 AM Jeroen Massar  wrote:
> >
[...]
> >
> > I personally would not touch the .h definitions:
> >
> > >
> > > Index: i386/include/cputypes.h
> > > [..]
> > >
> > > -#define  CPUCLASS_3860
> > > #define   CPUCLASS_4861
[...]
> > Though, one could argue for the include users that they should also be 
> > stripping their code of this use and super special casing...
> 
> How much of a real problem is this? These defines are ONLY available
> on the i386 platform.
> 
> Wouldn't you have to be doing something super unportable to begin with
> if you're using these defines for anything?

OTOH, if someone would want to do something super unportable, would
you tell them to use some other operating system?

I guess it is ok to drop supporting code but...

I think it would be prudent to keep those few old defines, just mark
them with relevant comment saying they are no longer being used and
only kept to prevent someone clever from redefining those values for
whatever clever purpose they will see in a future.

I believe it is still possible to buy 386 in a form of so called
industrial computer. Their life is expected to be decades
long. Someone out there might be supporting this old stuff while
making sure his code compiles on newer hardware, too. While not having
the defines in *.h should be ok, it does not really cost much to keep
them.

-- 
Regards,
Tomasz Rola

--
** A C programmer asked whether computer had Buddha's nature.  **
** As the answer, master did "rm -rif" on the programmer's home**
** directory. And then the C programmer became enlightened...  **
** **
** Tomasz Rola  mailto:tomasz_r...@bigfoot.com **