Re: prevent re-upgrade in powerpc64 boot loader
Visa Hankala wrote:
> On Sat, Sep 23, 2023 at 02:26:18PM +, Klemens Nanni wrote:
> > On Sat, Sep 23, 2023 at 01:11:32PM +0200, Mark Kettenis wrote:
> > > > Date: Thu, 21 Sep 2023 22:30:01 +
> > > > From: Klemens Nanni
> > > >
> > > > In comparison to MI boot which only cares about /bsd.upgrade's x bit,
> > > > powerpc64 rdboot just wants a regular file.
> > > >
> > > > Require and strip u+x before execution to prevent sysupgrade(8) loop.
> > > > I'm new to powerpc64 and can't think of a reason to be different.
> > > >
> > > > Feedback? Objection? OK?
> > >
> > > So there is a problem with this approach. Calling chmod() will mean
> > > the bootloader will change the filesystem. What happens if you're
> > > booting with a root filesystem that was not cleanly unmounted?
> > > Modifying a forcibly mounted filesystem may not be without risk.
> >
> > Isn't that already the case with chmo() /etc/random.seed?
> >
> > Can you explain how that is not a problem in other non-kernel boot loaders
> > using libsa's ufs2_open() instead of mount(2)?
>
> chmod() through libsa's filesystem code modifies only the inode.
Yes, it is very minimal. I studied the code, and as far as I could see one
block is written back.
> Doing a mount-chmod-unmount cycle using the kernel triggers multiple
> writes to the filesystem.
For ffs, around 20 blocks if I studied it right. For libsa, I think 3
blocks are written back.
Majority of those are rewrites of recently read blocks.
I think the risk is overplayed, and we haven't heard of real damage have
we?
> In the past, I have pondered if octeon (and powerpc64) bootcode should
> use libsa instead of replicating the MI boot code. I did not use libsa
> initially because the libsa and libc APIs differ, and I did not want
> to use custom system call stubs. The original octboot/kexec interface
> was not suitable for libsa use, either.
Correct, I remember this discussion. It would be complicated interfacing
to the stupid libsa interface from anywhere else. It's not just libsa that
is the problem, but a new independent consumer. It will be fragile.
> However, the libsa and libc worlds can be reconciled with a trick.
> The libsa code is first compiled in standalone mode into a relinkable
> object ("saboot"). This object is then tweaked to avoid name conflicts
> with libc. Finally, the object is linked with glue code and libc into
> a proper rdboot executable that the kernel can run.
>
> Some have seen this before.
Yes. And I still worry about it. You still have a new application linked
against crappy libsa.
Next, someone changes libsa for amd64, the only architecture as far as they
are concerned, ok maybe they check arm64 also. But they don't ensure that
you get the memo. (Whoever does that change doesn't even *consider* the
chance that their code will affect another architecture, it's bad -- they
don't even mentally process that there are other architectures in the tree).
saboot continues to compile, but the rules of libsa have changed, and the
consequences are undefined.
But if you stick with ffs, those kind of yahoo operations are more rare.
So I'm not sure of the tradeoff.
Re: Please test; midi(4): make midi{read,write}_filtops mp safe
On Sun, Sep 24, 2023 at 11:03:54PM +0300, Vitaliy Makkoveev wrote:
> Please test this diff, I have no midi(4) devices.
>
> midi(4) already uses `audio_lock' mutex(9) for filterops, but they are
> still kernel locked. Wipe out old selwakeup API and make them MP safe.
> knote_locked(9) will not grab kernel lock, so call it directly from
> interrupt handlers instead of scheduling software interrupts.
https://marc.info/?l=openbsd-tech&m=167604232828221 has minor takeaways
if you pay attention.
> Index: sys/dev/midi.c
> ===
> RCS file: /cvs/src/sys/dev/midi.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 midi.c
> --- sys/dev/midi.c2 Jul 2022 08:50:41 - 1.55
> +++ sys/dev/midi.c24 Sep 2023 19:57:56 -
> @@ -31,7 +31,6 @@
> #include
> #include
>
> -#define IPL_SOFTMIDI IPL_SOFTNET
> #define DEVNAME(sc) ((sc)->dev.dv_xname)
>
> int midiopen(dev_t, int, int, struct proc *);
> @@ -65,41 +64,38 @@ struct cfdriver midi_cd = {
>
> void filt_midiwdetach(struct knote *);
> int filt_midiwrite(struct knote *, long);
> +int filt_midimodify(struct kevent *, struct knote *);
> +int filt_midiprocess(struct knote *, struct kevent *);
>
> const struct filterops midiwrite_filtops = {
> - .f_flags= FILTEROP_ISFD,
> + .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
> .f_attach = NULL,
> .f_detach = filt_midiwdetach,
> .f_event= filt_midiwrite,
> + .f_modify = filt_midimodify,
> + .f_process = filt_midiprocess,
> };
>
> void filt_midirdetach(struct knote *);
> int filt_midiread(struct knote *, long);
>
> const struct filterops midiread_filtops = {
> - .f_flags= FILTEROP_ISFD,
> + .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
> .f_attach = NULL,
> .f_detach = filt_midirdetach,
> .f_event= filt_midiread,
> + .f_modify = filt_midimodify,
> + .f_process = filt_midiprocess,
> };
>
> void
> -midi_buf_wakeup(void *addr)
> +midi_buf_wakeup(struct midi_buffer *buf)
> {
> - struct midi_buffer *buf = addr;
> -
> if (buf->blocking) {
> wakeup(&buf->blocking);
> buf->blocking = 0;
> }
> - /*
> - * As long as selwakeup() grabs the KERNEL_LOCK() make sure it is
> - * already held here to avoid lock ordering problems with `audio_lock'
> - */
> - KERNEL_ASSERT_LOCKED();
> - mtx_enter(&audio_lock);
> - selwakeup(&buf->sel);
> - mtx_leave(&audio_lock);
> + knote_locked(&buf->klist, 0);
> }
>
> void
> @@ -117,13 +113,7 @@ midi_iintr(void *addr, int data)
>
> MIDIBUF_WRITE(mb, data);
>
> - /*
> - * As long as selwakeup() needs to be protected by the
> - * KERNEL_LOCK() we have to delay the wakeup to another
> - * context to keep the interrupt context KERNEL_LOCK()
> - * free.
> - */
> - softintr_schedule(sc->inbuf.softintr);
> + midi_buf_wakeup(mb);
> }
>
> int
> @@ -226,14 +216,7 @@ void
> midi_out_stop(struct midi_softc *sc)
> {
> sc->isbusy = 0;
> -
> - /*
> - * As long as selwakeup() needs to be protected by the
> - * KERNEL_LOCK() we have to delay the wakeup to another
> - * context to keep the interrupt context KERNEL_LOCK()
> - * free.
> - */
> - softintr_schedule(sc->outbuf.softintr);
> + midi_buf_wakeup(&sc->outbuf);
> }
>
> void
> @@ -342,11 +325,11 @@ midikqfilter(dev_t dev, struct knote *kn
> error = 0;
> switch (kn->kn_filter) {
> case EVFILT_READ:
> - klist = &sc->inbuf.sel.si_note;
> + klist = &sc->inbuf.klist;
> kn->kn_fop = &midiread_filtops;
> break;
> case EVFILT_WRITE:
> - klist = &sc->outbuf.sel.si_note;
> + klist = &sc->outbuf.klist;
> kn->kn_fop = &midiwrite_filtops;
> break;
> default:
> @@ -355,9 +338,7 @@ midikqfilter(dev_t dev, struct knote *kn
> }
> kn->kn_hook = (void *)sc;
>
> - mtx_enter(&audio_lock);
> - klist_insert_locked(klist, kn);
> - mtx_leave(&audio_lock);
> + klist_insert(klist, kn);
> done:
> device_unref(&sc->dev);
> return error;
> @@ -368,24 +349,15 @@ filt_midirdetach(struct knote *kn)
> {
> struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
>
> - mtx_enter(&audio_lock);
> - klist_remove_locked(&sc->inbuf.sel.si_note, kn);
> - mtx_leave(&audio_lock);
> + klist_remove(&sc->inbuf.klist, kn);
> }
>
> int
> filt_midiread(struct knote *kn, long hint)
> {
> struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
> - int retval;
> -
> - if ((hint & NOTE_SUBMIT) == 0)
> - mtx_enter(&audio_lock);
> - retval = !MIDIBUF_ISEMPTY(&sc->inbuf);
> - if ((hint & NOTE_SUBMIT) == 0)
> - mtx_leave(&a
Re: prevent re-upgrade in powerpc64 boot loader
On Sat, Sep 23, 2023 at 02:26:18PM +, Klemens Nanni wrote:
> On Sat, Sep 23, 2023 at 01:11:32PM +0200, Mark Kettenis wrote:
> > > Date: Thu, 21 Sep 2023 22:30:01 +
> > > From: Klemens Nanni
> > >
> > > In comparison to MI boot which only cares about /bsd.upgrade's x bit,
> > > powerpc64 rdboot just wants a regular file.
> > >
> > > Require and strip u+x before execution to prevent sysupgrade(8) loop.
> > > I'm new to powerpc64 and can't think of a reason to be different.
> > >
> > > Feedback? Objection? OK?
> >
> > So there is a problem with this approach. Calling chmod() will mean
> > the bootloader will change the filesystem. What happens if you're
> > booting with a root filesystem that was not cleanly unmounted?
> > Modifying a forcibly mounted filesystem may not be without risk.
>
> Isn't that already the case with chmo() /etc/random.seed?
>
> Can you explain how that is not a problem in other non-kernel boot loaders
> using libsa's ufs2_open() instead of mount(2)?
chmod() through libsa's filesystem code modifies only the inode.
Doing a mount-chmod-unmount cycle using the kernel triggers multiple
writes to the filesystem.
In the past, I have pondered if octeon (and powerpc64) bootcode should
use libsa instead of replicating the MI boot code. I did not use libsa
initially because the libsa and libc APIs differ, and I did not want
to use custom system call stubs. The original octboot/kexec interface
was not suitable for libsa use, either.
However, the libsa and libc worlds can be reconciled with a trick.
The libsa code is first compiled in standalone mode into a relinkable
object ("saboot"). This object is then tweaked to avoid name conflicts
with libc. Finally, the object is linked with glue code and libc into
a proper rdboot executable that the kernel can run.
Some have seen this before.
diff --git a/sys/arch/octeon/stand/Makefile b/sys/arch/octeon/stand/Makefile
index 498bd2809c6..4a86a8bef6b 100644
--- a/sys/arch/octeon/stand/Makefile
+++ b/sys/arch/octeon/stand/Makefile
@@ -1,5 +1,5 @@
# $OpenBSD: Makefile,v 1.4 2019/08/04 08:53:14 visa Exp $
-SUBDIR+= rdboot boot
+SUBDIR+= saboot rdboot boot
.include
diff --git a/sys/arch/octeon/stand/Makefile.inc
b/sys/arch/octeon/stand/Makefile.inc
deleted file mode 100644
index 18d8470f888..000
--- a/sys/arch/octeon/stand/Makefile.inc
+++ /dev/null
@@ -1,45 +0,0 @@
-# $OpenBSD: Makefile.inc,v 1.1 2013/06/05 01:02:29 jasper Exp $
-
-BINDIR=/usr/mdec
-
-STANDALONE?= -D_STANDALONE
-
-.if ${MACHINE} == "octeon"
-CPPFLAGS+= ${STANDALONE}
-CPPFLAGS+= -I.
-CFLAGS+= -fno-stack-protector -Wall
-CFLAGS+= -fno-builtin-vprintf -fno-builtin-printf -fno-builtin-putchar
-# Silence warnings
-CFLAGS+= -fno-builtin-snprintf
-CFLAGS+= -fno-builtin-memcpy
-CFLAGS+= -fno-builtin-memcmp
-CFLAGS+= -fno-builtin-memset
-CFLAGS+= -fno-builtin-strncpy
-CFLAGS+= -fno-builtin-strncmp
-CFLAGS+= -fno-builtin-exit
-SAABI= -mips3 -mno-abicalls -G 0 -fno-pic -fno-common
-AS?= as
-LD?= ld
-.endif
-
-### Figure out what to use for libsa
-LIBSADIR?= ${.CURDIR}/../libsa
-
-.if exists(${LIBSADIR}/${__objdir})
-LIBSAOBJDIR=${LIBSADIR}/${__objdir}
-.else
-LIBSAOBJDIR=${LIBSADIR}
-.endif
-
-LIBSA= ${LIBSAOBJDIR}/libsa.a
-
-### Figure out what to use for libz
-LIBZDIR?=${.CURDIR}/../libz
-
-.if exists(${LIBZDIR}/${__objdir})
-LIBZOBJDIR= ${LIBZDIR}/${__objdir}
-.else
-LIBZOBJDIR= ${LIBZDIR}
-.endif
-
-LIBZ= ${LIBZOBJDIR}/libz.a
diff --git a/sys/arch/octeon/stand/libsa/Makefile
b/sys/arch/octeon/stand/libsa/Makefile
deleted file mode 100644
index 3b2961f25c0..000
--- a/sys/arch/octeon/stand/libsa/Makefile
+++ /dev/null
@@ -1,51 +0,0 @@
-# $OpenBSD: Makefile,v 1.8 2019/10/29 02:55:52 deraadt Exp $
-
-.include "${.CURDIR}/../Makefile.inc"
-
-LIB= sa
-
-.PATH: ${.CURDIR}/../../../../lib/libsa
-
-CLEANFILES += machine mips64
-
-CFLAGS+= ${CEXTRAFLAGS} ${SAABI} -nostdinc -mno-abicalls -D_NO_ABICALLS \
- -fno-pie \
- -I${.CURDIR} -I${.CURDIR}/../include -I${.CURDIR}/../.. \
- -I${.CURDIR}/../../.. -I${.CURDIR}/../../../.. \
- -I${.CURDIR}/../../../../lib/libsa \
- -I${.OBJDIR}
-
-# stand routines
-SRCS= alloc.c cons.c ctime.c exit.c getchar.c getfile.c getln.c globals.c \
- memcmp.c memcpy.c memmove.c memset.c printf.c putchar.c \
- snprintf.c strchr.c strcmp.c strerror.c strncmp.c strncpy.c strtol.c
-
-# io routines
-SRCS+= close.c closeall.c dev.c disklabel.c dkcksum.c fstat.c ioctl.c \
- lseek.c open.c read.c readdir.c stat.c write.c
-
-#SRCS+=cread.c
-#CPPFLAGS+= -D__INTERNAL_LIBSA_CREAD
-
-# boot filesystems
-SRCS+= ufs.c cd9660.c
-
-CFLAGS+=-DNO_NET
-
-SRCS+= loadfile.c arc4.c
-
-${OBJS}: ${.CURDIR}/../Makefile.inc
-
-NOPROFILE=
-NOPIC=
-
-.if !make(
Disklabel Spoofing and the GPT "Required" Partition Attribute
Hi, So I was using OpenBSD on a USB thumb drive to have a look at the EFI system partition of two laptops (one Windows 10, one Windows 11) when I realized that I couldn't mount the EFI system partition of the Windows 11 laptop at all because there simply wasn't any partition letter defined for it. Here's the (partial) output of fdisk -v on that laptop: Primary GPT: Disk: sd0 Usable LBA: 34 to 2000409230 [2000409264 Sectors] GUID: da4367e3-0319-487a-a475-79535378e9ef #: type [ start: size ] guid name 0: EFI Sys [2048: 532480 ] 10aaba25-0b31-4784-a742-47744e0d1e95 EFI system partition Attributes: (0x8001) Required MSNoAutoMount 1: e3c9e316-0b5c-4db8-817d-f92df00215ae [ 534528:32768 ] ec2ce10b-f72e-49a4-abac-9cb519913748 Microsoft reserved partition 2: Microsoft basic data [ 567296: 1995745280 ] 9422ea37-94b2-4692-89c1-0d4e28b28acb Basic data partition 3: Win Recovery [ 1996312576: 4096000 ] ceffc3ac-538e-42c7-b5fc-23200927e300 Basic data partition Attributes: (0x8001) Required MSNoAutoMount and the (partial) output of disklabel: 16 partitions: #size offset fstype [fsize bsize cpg] c: 20004092640 unused i:32768 534528 unknown j: 1995745280 567296 MSDOS As you can see, the EFI system partition of that laptop has the "Required" attribute. Digging some more, I found these slides by Ken Westerback: https://www.openbsd.org/papers/eurobsdcon2022-krw-blockstobooting.pdf which confirm (slide 22) that the OpenBSD kernel doesn't "spoof GPT partitions with 'Required' attribute" anymore. That's also confirmed by the cvs log message for revision 1.261 of src/sys/kern/subr_disk.c: Add #define's for GPT partition attribute bits REQUIRED, IGNORE and BOOTABLE, set BOOTABLE attribute bit instead of using the incorrect GPTDOSACTIVE value, have 'fdisk -v' print out GPT partition attributes if any of the 64 bits are set, don't spoof any partition with REQUIRED bit set. Prompted by kettenis@ stumbling across a machine with 40+ (!!) REQUIRED GPT partitions. So... now what? I'm in a situation where I can't mount a FAT partition, and as far as I know there's no OpenBSD tool to manually edit GPT partition attributes either, so I'm stuck. The explanation from the cvs log is clear but would it be possible to not spoof "Required" partitions only on arm64 rather than all platforms (arm64 being the "machine" described in the cvs log message, according to the video of Ken Westerback's presentation)? Or maybe use the MSNoAutoMount attribute to undo the effects of the Required one with regard to spoofing? Philippe
Re: vmt(4): use shared netlock to protect ifnet data within vmt_tclo_broadcastip()
On Sat, 23 Sep 2023 15:38:40 +0300
Vitaliy Makkoveev wrote:
> This makes ifnet protection consistent. Execute vmt_tclo_tick() timeout
> handler in process context to allow context switch within
> vmt_tclo_broadcastip().
ok yasuoka
> Index: sys/dev/pv/vmt.c
> ===
> RCS file: /cvs/src/sys/dev/pv/vmt.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 vmt.c
> --- sys/dev/pv/vmt.c 7 Jan 2023 06:40:21 - 1.30
> +++ sys/dev/pv/vmt.c 23 Sep 2023 12:15:27 -
> @@ -471,7 +471,7 @@ vmt_attach(struct device *parent, struct
>
> config_mountroot(self, vmt_tick_hook);
>
> - timeout_set(&sc->sc_tclo_tick, vmt_tclo_tick, sc);
> + timeout_set_proc(&sc->sc_tclo_tick, vmt_tclo_tick, sc);
> timeout_add_sec(&sc->sc_tclo_tick, 1);
> sc->sc_tclo_ping = 1;
>
> @@ -899,9 +899,12 @@ vmt_tclo_broadcastip(struct vmt_softc *s
> {
> struct ifnet *iface;
> struct sockaddr_in *guest_ip;
> + char ip[INET_ADDRSTRLEN];
>
> /* find first available ipv4 address */
> guest_ip = NULL;
> +
> + NET_LOCK_SHARED();
> TAILQ_FOREACH(iface, &ifnetlist, if_list) {
> struct ifaddr *iface_addr;
>
> @@ -918,14 +921,14 @@ vmt_tclo_broadcastip(struct vmt_softc *s
> continue;
>
> guest_ip = satosin(iface_addr->ifa_addr);
> + inet_ntop(AF_INET, &guest_ip->sin_addr, ip,
> + sizeof(ip));
> break;
> }
> }
> + NET_UNLOCK_SHARED();
>
> if (guest_ip != NULL) {
> - char ip[INET_ADDRSTRLEN];
> -
> - inet_ntop(AF_INET, &guest_ip->sin_addr, ip, sizeof(ip));
> if (vm_rpc_send_rpci_tx(sc, "info-set guestinfo.ip %s",
> ip) != 0) {
> DPRINTF("%s: unable to send guest IP address\n",
[POC] external UTF-8 validation for mail(1)
Following on from the "Send international text with mail(1)" thread...
There is some interest in making mail(1) add relevant MIME headers to allow:
* Correctly sending UTF-8 email.
* Identifying 7-bit ASCII emails with an appropriate content type.
... and possibly other things in the future.
However:
* Adding UTF-8 parsing directly in mail(1) and hard-coding it's behaviour is
inflexible.
* Exactly what should be sent with UTF-8 headers rather than none or us-ascii
is partly down to personal preference:
1. Send everything as UTF-8, including plain ASCII.
2. Send nothing as UTF-8.
3. Send valid UTF-8 streams as UTF-8, everything else ASCII.
4. Send valid and sort-of-valid-but-not-really UTF-8 streams as UTF-8, and
plain ASCII as us-ascii.
... etc, etc.
Different sites might reasonably have different requirements. Plus, we don't
really want to break mail(1) for anybody.
As can be seen from the size of the previous thread, finding a universal
solution that suits everybody has not yet been possible.
In an attempt to solve this, I've produced a proof of concept patch for
mail(1) to allow it to call a fixed external program, passing the mail to it
on standard input for analysis and receiving back a flag to indicate which set
of MIME headers should be included.
This is only a POC at this stage, so there may be bugs and room for
improvement. But it seems to work.
Advantages of this approach:
* Very minimal changes to mail(1).
* Flexible.
* No change for people who don't want this functionality.
- If the external validator program is not installed, mail(1) does not add
any new headers at all.
Cheat sheet:
1. Apply the patch, re-compile and re-install mail(1).
2. Compile the new program and put it in /bin/validate_utf8 .
3. Send mail with mail(1) and observe the headers.
For ease of testing by users who don't use or care about UTF-8, the demo
validator simply looks for an 'X' character in the mail body, and if it finds
one then it treats the mail as UTF-8, everything else is treated as us-ascii.
A real UTF-8 validator would return 2 for a valid UTF-8 stream, 1 for ASCII,
and 0 for non-conformant data that we don't want to mess with, (E.G. a legacy
8-bit encoding).
Have fun!
--- collect.c.dist Fri Jan 17 15:42:30 2014
+++ collect.c Sun Sep 24 19:09:04 2023
@@ -39,6 +39,7 @@
#include "rcv.h"
#include "extern.h"
+#include
/*
* Read a message from standard output and return a read file to it
@@ -62,6 +63,12 @@
char getsub;
char linebuf[LINESIZE], tempname[PATHSIZE], *cp;
+ int val_status;
+ sigset_t old_sigmask;
+ sigset_t temp_sigmask;
+ pid_t pid;
+ #define VALIDATOR "/bin/validate_utf8"
+
collf = NULL;
eofcount = 0;
hadintr = 0;
@@ -374,7 +381,77 @@
(void)Fclose(collf);
collf = NULL;
}
+
out:
+
+/*
+ * Pass the content of the collected file to stdin of a forked
+ * validator program, and use it's exit status to set a flag
+ * in the struct header that we can later use to include an
+ * appropriate content type header.
+ */
+
+rewind(collf);
+
+/*
+ * If this fork fails, it's not a critical error. We just don't
+ * perform any UTF-8 validation in that case.
+ */
+
+pid=fork();
+if (pid==-1) {
+ goto done;
+ }
+
+if (pid==0) {
+ sigset_t val_sigs;
+ sigemptyset(&val_sigs);
+ sigaddset(&val_sigs, SIGHUP);
+ prepare_child(&val_sigs, fileno(collf), -1);
+ execl(VALIDATOR, VALIDATOR, NULL);
+ /*
+* If the validator doesn't exist or isn't executable then
+* the following exit value will be passed to the parent
+* below. Therefore, it must _not_ conflict with an
+* expected exit value from the validator.
+*/
+ _exit(127);
+ }
+
+/*
+ * To wait on the forked validator and get it's exit status we need
+ * to enable SIGCHLD.
+ */
+
+sigemptyset(&temp_sigmask);
+sigaddset(&temp_sigmask, SIGCHLD);
+sigprocmask(SIG_BLOCK, &temp_sigmask, &old_sigmask);
+if (waitpid(pid, &val_status, 0) != -1) {
+ if (WIFEXITED(val_status)) {
+ /*
+* Only permit _specific_ values.
+*/
+ if (WEXITSTATUS(val_status) != 127)
+ fprintf (stderr, "Validator %s returned status %d\n",
+ VALIDATOR, WEXITSTATUS(val_status));
+ if (WEXITSTATUS(val_status)==1)
+ hp->enc_flag=1;
+ if (WEXITSTATUS(val_status)==2)
+ hp->enc_flag=2;
+ }
+ }
+
+/*
+ * Restore previous signal mask now that we are done with the validator.
+ */
+
+sigprocmask(SIG_SETMASK, &old_sigmask, NULL);
+
+/*
+ * All done!
+ */
+
+done:
if (collf != NULL)
rewind(collf);
noreset--;
--- def.h.dist Fri Jan 28 03:18:41 2022
+++ def.h Sun Sep 24 16:01:03 2023
@@ -176,6 +176,7 @@
struct name
Viable ROP-free roadmap for i386/armv8/riscv64/alpha/sparc64
20-some years ago I was very much involved in the integration of the
stack-protector into OpenBSD. This subsystem was developed as a gcc
patch by Hiroaki Etoh. Many years later it adopted and substantially
rewritten to incorporate it into mainline gcc. Thus, OpenBSD for a few
years was the first & only system with the stack protector.
Miod Vallat, Dale Rahn, (some forgotten), and I incorporated the code
into OpenBSD, fixed many problems with Etoh, and made some decisions
along the way.
One of these decisions was that the original stack protector protected
all functions. But this was too expensive. gcc at that time did not
know the types of various local variables were (to for instance, look
for character arrays). It only knew the total size. So it was I who
chose the value of 16, which has infected the ecosystem for 2 decades.
Only functions with 16 or more bytes of local storage are protected.
Another decision was where the stack protector check function was
located. We placed it into libc. And then we re-wrote it very
carefully to be reentrant and safe in all calling conditions; the
original proposal was not clean.
Originally there was only one cookie per program, but Matthew Dempsky
made changes so that every DSO (shared library object) had a different
cookie. So a program like ssh would have 6 cookies, and far more than
that in a dynamically linked browser. If you crash in one DSO and get
visibility of your own cookie, that doesn't help you do function calls
in another DSO (for example libc.so).
Via Matthew Dempsky and others, I provided ideas for better heuristic
to select functions to protect rather than the 16-byte heuristic, and
eventually some people at google wrote the -fstack-protector-strong
diffs for gcc and clang, because modern compilers keep better track of
the types and format of their local variables.
Years later, Todd Mortimer and I developed RETGUARD. At the start of
that initiative he proposed we protect all functions, to try to guard
all the RET instructions, and therefore achieve a state we call
"ROP-free". I felt this was impossible, but after a couple hurdles the
RETGUARD performance was vastly better than the stack protector and we
were able to protect all functions and get to ROP-free (on fixed-sized
instruction architecures). Performance was acceptable to trade against
improved security.
On variable-sized instruction architectures, polymorphic RET and other
control flow instructions can and will surface, but the available RET
gadgets are seriously reduced and exploitation may not be possible.
Other methods attempt to reduce the impact of the poly-gadgets. Still
the effort has value on all architectures. So amd64 isn't as good as
arm64, riscv64, mips64, powerpc, or powerpc64.
RETGUARD provides up to 4096 cookies per DSO, per-function, but limited to
avoid excessive bloat. It is difficult to do on architectures with very
few registers. Code was only written for clang, there is no gcc codebase
doing it. clang code for some architectures was never written (riscv64).
I hope that sets the stage for what is coming next.
We were able to enable RETGUARD on all functions because it was fast.
Why is RETGUARD fast, and the stack protector slow? In pseudo-code, the
OpenBSD stack-protector model creates function prologue and epilogue which
look like this:
{
local local_saved_cookie = per_DSO_cookie;
if (per_DSO_cookie != local_saved_cooke)
__stack_smash_handler(name_of_function)
}
This issues a warning to stdout, then crashes a manual SIGABRT, and you
can use the debugger on the core file in the wrong frame (you are inside
__stack_smash_handler, not at the momemt of the fault).
RETGUARD made a choice to not use a smash-reporting function, and instead
does:
{
local
if (retguard_matches>
illegal-instruction
}
So a detected-corruption causes an immediate crash, generally with a
SIGABRT, you get no detailed report. But you can use the debugger on the
core file in exactly the correct frame (an improvement).
At first glance RETGUARD is faster because it has less instructions.
But remember we are now living in a speculative-execution universe. A
few years ago some speculation reseachers I talked to pointed out that
the stack-protector generated instructions to do the call into
__stack_smash_handler(), and even many instructions inside the function
itself, are fetched, decoded, issued, and their results are discarded.
That's a waste of cpu resources. It might be a slowdown because those
execution slots are not used exclusively for straight-line speculation
following the RET. Modern cpus also have complicated branch-target
caches which may not be heuristically tuned to the stack protector approach.
On the other hand the RETGUARD approach uses an illegal instruction (of
some sort), which is a speculation barrier. That prevents the cpu from
heading off into an alterna
Please test; midi(4): make midi{read,write}_filtops mp safe
Please test this diff, I have no midi(4) devices.
midi(4) already uses `audio_lock' mutex(9) for filterops, but they are
still kernel locked. Wipe out old selwakeup API and make them MP safe.
knote_locked(9) will not grab kernel lock, so call it directly from
interrupt handlers instead of scheduling software interrupts.
Index: sys/dev/midi.c
===
RCS file: /cvs/src/sys/dev/midi.c,v
retrieving revision 1.55
diff -u -p -r1.55 midi.c
--- sys/dev/midi.c 2 Jul 2022 08:50:41 - 1.55
+++ sys/dev/midi.c 24 Sep 2023 19:57:56 -
@@ -31,7 +31,6 @@
#include
#include
-#define IPL_SOFTMIDI IPL_SOFTNET
#define DEVNAME(sc)((sc)->dev.dv_xname)
intmidiopen(dev_t, int, int, struct proc *);
@@ -65,41 +64,38 @@ struct cfdriver midi_cd = {
void filt_midiwdetach(struct knote *);
int filt_midiwrite(struct knote *, long);
+int filt_midimodify(struct kevent *, struct knote *);
+int filt_midiprocess(struct knote *, struct kevent *);
const struct filterops midiwrite_filtops = {
- .f_flags= FILTEROP_ISFD,
+ .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
.f_attach = NULL,
.f_detach = filt_midiwdetach,
.f_event= filt_midiwrite,
+ .f_modify = filt_midimodify,
+ .f_process = filt_midiprocess,
};
void filt_midirdetach(struct knote *);
int filt_midiread(struct knote *, long);
const struct filterops midiread_filtops = {
- .f_flags= FILTEROP_ISFD,
+ .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
.f_attach = NULL,
.f_detach = filt_midirdetach,
.f_event= filt_midiread,
+ .f_modify = filt_midimodify,
+ .f_process = filt_midiprocess,
};
void
-midi_buf_wakeup(void *addr)
+midi_buf_wakeup(struct midi_buffer *buf)
{
- struct midi_buffer *buf = addr;
-
if (buf->blocking) {
wakeup(&buf->blocking);
buf->blocking = 0;
}
- /*
-* As long as selwakeup() grabs the KERNEL_LOCK() make sure it is
-* already held here to avoid lock ordering problems with `audio_lock'
-*/
- KERNEL_ASSERT_LOCKED();
- mtx_enter(&audio_lock);
- selwakeup(&buf->sel);
- mtx_leave(&audio_lock);
+ knote_locked(&buf->klist, 0);
}
void
@@ -117,13 +113,7 @@ midi_iintr(void *addr, int data)
MIDIBUF_WRITE(mb, data);
- /*
-* As long as selwakeup() needs to be protected by the
-* KERNEL_LOCK() we have to delay the wakeup to another
-* context to keep the interrupt context KERNEL_LOCK()
-* free.
-*/
- softintr_schedule(sc->inbuf.softintr);
+ midi_buf_wakeup(mb);
}
int
@@ -226,14 +216,7 @@ void
midi_out_stop(struct midi_softc *sc)
{
sc->isbusy = 0;
-
- /*
-* As long as selwakeup() needs to be protected by the
-* KERNEL_LOCK() we have to delay the wakeup to another
-* context to keep the interrupt context KERNEL_LOCK()
-* free.
-*/
- softintr_schedule(sc->outbuf.softintr);
+ midi_buf_wakeup(&sc->outbuf);
}
void
@@ -342,11 +325,11 @@ midikqfilter(dev_t dev, struct knote *kn
error = 0;
switch (kn->kn_filter) {
case EVFILT_READ:
- klist = &sc->inbuf.sel.si_note;
+ klist = &sc->inbuf.klist;
kn->kn_fop = &midiread_filtops;
break;
case EVFILT_WRITE:
- klist = &sc->outbuf.sel.si_note;
+ klist = &sc->outbuf.klist;
kn->kn_fop = &midiwrite_filtops;
break;
default:
@@ -355,9 +338,7 @@ midikqfilter(dev_t dev, struct knote *kn
}
kn->kn_hook = (void *)sc;
- mtx_enter(&audio_lock);
- klist_insert_locked(klist, kn);
- mtx_leave(&audio_lock);
+ klist_insert(klist, kn);
done:
device_unref(&sc->dev);
return error;
@@ -368,24 +349,15 @@ filt_midirdetach(struct knote *kn)
{
struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
- mtx_enter(&audio_lock);
- klist_remove_locked(&sc->inbuf.sel.si_note, kn);
- mtx_leave(&audio_lock);
+ klist_remove(&sc->inbuf.klist, kn);
}
int
filt_midiread(struct knote *kn, long hint)
{
struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
- int retval;
-
- if ((hint & NOTE_SUBMIT) == 0)
- mtx_enter(&audio_lock);
- retval = !MIDIBUF_ISEMPTY(&sc->inbuf);
- if ((hint & NOTE_SUBMIT) == 0)
- mtx_leave(&audio_lock);
- return (retval);
+ return (!MIDIBUF_ISEMPTY(&sc->inbuf));
}
void
@@ -393,24 +365,39 @@ filt_midiwdetach(struct knote *kn)
{
struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
- mtx_enter(&audio_lock);
- klist_remove_locked(&sc->outb
dhcpd(8): Parse lease database after dropping privileges
Hello,
While reading dhcpd's code, I noticed it parses the lease database
file ("/var/db/dhcpd.leases") while the process is running as root.
This happens prior to switching to the "_dhcp" user and calling
chroot(2) / pledge(2).
This is potentially unsafe because the lease database file contains
attacker-controlled data (such as client hostnames). I really like
the thoughtful security measures that the daemon takes on startup.
It felt worthwhile to ensure that the parsing logic executes after
those measures are applied.
This patch attempts to split the lease database file logic apart,
making dhcpd behave like this:
1. Open read-only and write-only file descriptors to the lease DB
2. Drop privileges / apply security policy
3. Read data from the read-only file descriptor and parse the data
The tricky part about this change is that the lease parsing logic
opens and reads from the following files:
- /usr/share/zoneinfo/GMT (Due to date string with timezone)
- /usr/share/zoneinfo/posixrules (Due to date string with timezone)
- /etc/ethers (Because of call to ether_hostton(3))
Although the first two files are covered by pledge(2) "stdio", the
ethers file is not. Another complication is that chroot(2) call
makes those files unavailable to the process.
To handle this complication, I decided to replace the chroot(2) call
with unveil(2). This required the addition of the "rpath" promise to
read ethers. I am not the biggest fan of the pledge(2) code I added.
Perhaps there is a cleaner / more graceful way to drop the
"rpath" privilege?
Best regards,
Stephen
---
usr.sbin/dhcpd/confpars.c | 41 ++-
usr.sbin/dhcpd/db.c | 19 ++
usr.sbin/dhcpd/dhcpd.c| 33 +--
usr.sbin/dhcpd/dhcpd.h| 2 ++
4 files changed, 79 insertions(+), 16 deletions(-)
diff --git usr.sbin/dhcpd/confpars.c usr.sbin/dhcpd/confpars.c
index 1bdffb38842..bb245fc663e 100644
--- usr.sbin/dhcpd/confpars.c
+++ usr.sbin/dhcpd/confpars.c
@@ -57,6 +57,8 @@
#include "dhctoken.h"
#include "log.h"
+FILE *db_file_ro;
+
intparse_cidr(FILE *, unsigned char *, unsigned char *);
/*
@@ -106,18 +108,11 @@ readconf(void)
}
/*
- * lease-file :== lease-declarations EOF
- * lease-statments :==
- *| lease-declaration
- *| lease-declarations lease-declaration
+ * Open and retain a read-only file descriptor to the lease database file.
*/
void
-read_leases(void)
+open_leases(void)
{
- FILE *cfile;
- char *val;
- int token;
-
new_parse(path_dhcpd_db);
/*
@@ -131,23 +126,40 @@ read_leases(void)
* thinking that no leases have been assigned to anybody, which
* could create severe network chaos.
*/
- if ((cfile = fopen(path_dhcpd_db, "r")) == NULL) {
+ if ((db_file_ro = fopen(path_dhcpd_db, "r")) == NULL) {
log_warn("Can't open lease database (%s)", path_dhcpd_db);
log_warnx("check for failed database rewrite attempt!");
log_warnx("Please read the dhcpd.leases manual page if you");
fatalx("don't know what to do about this.");
}
+}
+
+/*
+ * lease-file :== lease-declarations EOF
+ * lease-statments :==
+ *| lease-declaration
+ *| lease-declarations lease-declaration
+ */
+void
+read_leases(void)
+{
+ char *val;
+ int token;
+
+ if (!db_file_ro) {
+ fatalx("db_file_ro is NULL");
+ }
do {
- token = next_token(&val, cfile);
+ token = next_token(&val, db_file_ro);
if (token == EOF)
break;
if (token != TOK_LEASE) {
log_warnx("Corrupt lease file - possible data loss!");
- skip_to_semi(cfile);
+ skip_to_semi(db_file_ro);
} else {
struct lease *lease;
- lease = parse_lease_declaration(cfile);
+ lease = parse_lease_declaration(db_file_ro);
if (lease)
enter_lease(lease);
else
@@ -155,7 +167,8 @@ read_leases(void)
}
} while (1);
- fclose(cfile);
+ fclose(db_file_ro);
+ db_file_ro = NULL;
}
/*
diff --git usr.sbin/dhcpd/db.c usr.sbin/dhcpd/db.c
index 295e522b1ce..634ec8a593a 100644
--- usr.sbin/dhcpd/db.c
+++ usr.sbin/dhcpd/db.c
@@ -190,6 +190,16 @@ commit_leases(void)
return (1);
}
+/*
+ * Open and retain two file descriptors to the lease database file:
+ *
+ * - A read-only fd for learning existing leases
+ * - A write-only fd for writing new leases
+ *
+ * Reading and parsing data from the read-only fd is done separately.
+ * This allows privilege drop to happen *before* parsing untrusted
+ * client data from the lease database file.
+
Re: update bsd.regress.mk(5)
On Sun, Sep 24, 2023 at 04:22:30PM +0200, Theo Buehler wrote: > On Sun, Sep 24, 2023 at 03:17:11PM +0200, Claudio Jeker wrote: > > Try to document how REGRESS_LOG and REGRESS_FAIL_EARLY interact. > > We could make it fully precise with a few more words. > Done -- :wq Claudio Index: bsd.regress.mk.5 === RCS file: /cvs/src/share/man/man5/bsd.regress.mk.5,v retrieving revision 1.24 diff -u -p -r1.24 bsd.regress.mk.5 --- bsd.regress.mk.531 Mar 2022 17:27:22 - 1.24 +++ bsd.regress.mk.524 Sep 2023 16:58:58 - @@ -79,11 +79,20 @@ If this variable is set to anything but the .Cm regress target will abort as soon as a test fails. +Defaults to +.Dq yes +unless +.Ev REGRESS_LOG +is set. .It Ev REGRESS_LOG Points to the fully-qualified path of a file to which regression results are appended. Defaults to .Pa /dev/null . +If set to any other path, +.Ev REGRESS_FAIL_EARLY +defaults to +.Dq no . .It Ev REGRESS_ROOT_TARGETS Targets for which root access is required to run the test. The
Re: Send international text with mail(1) - proposal and patches
On Sun, Sep 24, 2023 at 11:12:10AM -0300, Crystal Kolipe wrote:
> On Sun, Sep 24, 2023 at 12:37:08PM +0200, Walter Alejandro Iglesias wrote:
> > +static int
> > +not_utf8(FILE *message, int len)
> > +{
> > + int n, ulen;
> > + unsigned char s[len];
>
> Please re-read Omar's advice about large unbounded arrays.
Better?
Index: send.c
===
RCS file: /cvs/src/usr.bin/mail/send.c,v
retrieving revision 1.26
diff -u -p -r1.26 send.c
--- send.c 8 Mar 2023 04:43:11 - 1.26
+++ send.c 24 Sep 2023 14:54:25 -
@@ -32,6 +32,11 @@
#include "rcv.h"
#include "extern.h"
+#include "locale.h"
+
+/* To check charset of the message and add the appropiate MIME headers */
+static char nutf8;
+static int not_utf8(FILE *s, int len);
static volatile sig_atomic_t sendsignal; /* Interrupted by a signal? */
@@ -341,6 +346,11 @@ mail1(struct header *hp, int printheader
else
puts("Null message body; hope that's ok");
}
+
+ /* Check non valid UTF-8 characters in the message */
+ nutf8 = not_utf8(mtf, fsize(mtf));
+ rewind(mtf);
+
/*
* Now, take the user names from the combined
* to and cc lists and do all the alias
@@ -520,15 +530,30 @@ puthead(struct header *hp, FILE *fo, int
gotcha = 0;
from = hp->h_from ? hp->h_from : value("from");
if (from != NULL)
- fprintf(fo, "From: %s\n", from), gotcha++;
+ fprintf(fo, "From: %s\n", from),
+ gotcha++;
if (hp->h_to != NULL && w & GTO)
- fmt("To:", hp->h_to, fo, w&GCOMMA), gotcha++;
+ fmt("To:", hp->h_to, fo, w&GCOMMA),
+ gotcha++;
if (hp->h_subject != NULL && w & GSUBJECT)
- fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
+ fprintf(fo, "Subject: %s\n", hp->h_subject),
+ gotcha++;
+ if (nutf8 == 0)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=us-ascii\n"
+ "Content-Transfer-Encoding: 7bit\n"),
+ gotcha++;
+ else if (nutf8 == 1)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=utf-8\n"
+ "Content-Transfer-Encoding: 8bit\n"),
+ gotcha++;
if (hp->h_cc != NULL && w & GCC)
- fmt("Cc:", hp->h_cc, fo, w&GCOMMA), gotcha++;
+ fmt("Cc:", hp->h_cc, fo, w&GCOMMA),
+ gotcha++;
if (hp->h_bcc != NULL && w & GBCC)
- fmt("Bcc:", hp->h_bcc, fo, w&GCOMMA), gotcha++;
+ fmt("Bcc:", hp->h_bcc, fo, w&GCOMMA),
+ gotcha++;
if (gotcha && w & GNL)
(void)putc('\n', fo);
return(0);
@@ -609,4 +634,44 @@ sendint(int s)
{
sendsignal = s;
+}
+
+/* Search non valid UTF-8 characters in the message */
+static int
+not_utf8(FILE *message, int len)
+{
+ int c, count, n, ulen;
+ size_t i, resize;
+ size_t jump = 100;
+ unsigned char *s = NULL;
+
+ setlocale(LC_CTYPE, "en_US.UTF-8");
+
+ if (s == NULL && (s = malloc(jump)) == NULL)
+ err(1, NULL);
+
+ i = count = 0;
+ while ((c = getc(message)) != EOF) {
+ if (s == NULL || count == jump) {
+ if ((s = realloc(s, i + jump + 1)) == NULL)
+ err(1, NULL);
+ count = 0;
+ }
+ s[i++] = c;
+ count++;
+ }
+
+ s[i] = '\0';
+
+ ulen = mbstowcs(NULL, s, 0);
+
+ if (ulen == len)
+ n = 0;
+ else if (ulen < 0)
+ n = 2;
+ else if (ulen < len)
+ n = 1;
+
+ free(s);
+ return n;
}
--
Walter
Re: update bsd.regress.mk(5)
On Sun, Sep 24, 2023 at 03:17:11PM +0200, Claudio Jeker wrote: > Try to document how REGRESS_LOG and REGRESS_FAIL_EARLY interact. We could make it fully precise with a few more words. > > -- > :wq Claudio > > Index: bsd.regress.mk.5 > === > RCS file: /cvs/src/share/man/man5/bsd.regress.mk.5,v > retrieving revision 1.24 > diff -u -p -r1.24 bsd.regress.mk.5 > --- bsd.regress.mk.5 31 Mar 2022 17:27:22 - 1.24 > +++ bsd.regress.mk.5 24 Sep 2023 13:15:27 - > @@ -79,11 +79,20 @@ If this variable is set to anything but > the > .Cm regress > target will abort as soon as a test fails. > +By default the value is I'd go with the shorter form used elsewhere: Defaults to > +.Dq yes > +unless > +.Ev REGRESS_LOG > +is set. > .It Ev REGRESS_LOG > Points to the fully-qualified path of a file to which regression > results are appended. > Defaults to > .Pa /dev/null . > +If set > +.Ev REGRESS_FAIL_EARLY > +defaults to > +.Dq no . I'd spill the words to make it precise: If set to any other path, .Ev REGRESS_FAIL_EARLY defaults to .Dq no . (not sure if grammar requires us to say "If REGRESS_LOG is set ...") > .It Ev REGRESS_ROOT_TARGETS > Targets for which root access is required to run the test. > The >
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: Send international text with mail(1) - proposal and patches
On Sun, Sep 24, 2023 at 12:37:08PM +0200, Walter Alejandro Iglesias wrote:
> +static int
> +not_utf8(FILE *message, int len)
> +{
> + int n, ulen;
> + unsigned char s[len];
Please re-read Omar's advice about large unbounded arrays.
update bsd.regress.mk(5)
Try to document how REGRESS_LOG and REGRESS_FAIL_EARLY interact. -- :wq Claudio Index: bsd.regress.mk.5 === RCS file: /cvs/src/share/man/man5/bsd.regress.mk.5,v retrieving revision 1.24 diff -u -p -r1.24 bsd.regress.mk.5 --- bsd.regress.mk.531 Mar 2022 17:27:22 - 1.24 +++ bsd.regress.mk.524 Sep 2023 13:15:27 - @@ -79,11 +79,20 @@ If this variable is set to anything but the .Cm regress target will abort as soon as a test fails. +By default the value is +.Dq yes +unless +.Ev REGRESS_LOG +is set. .It Ev REGRESS_LOG Points to the fully-qualified path of a file to which regression results are appended. Defaults to .Pa /dev/null . +If set +.Ev REGRESS_FAIL_EARLY +defaults to +.Dq no . .It Ev REGRESS_ROOT_TARGETS Targets for which root access is required to run the test. The
adding support for Meinberg PZF180PEX
Hi,
The following diff adds support for the Meinberg PZF180PEX card.
With this diff, the card is recognized and functions as expected:
mbg0 at pci13 dev 0 function 0 "Meinberg Funkuhren PZF180PEX" rev 0x01:
synchronized
$ ntpctl -s S
sensor
wt gd st next poll offset correction
mbg0 DCFp
1 1 09s 15s 0.014ms 0.000ms
(full dmesg below)
Would it be possible to add this to the tree?
Thanks and best regards,
Maurice Janssen
Index: sys/arch/amd64//conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.518
diff -u -p -u -p -r1.518 GENERIC
--- sys/arch/amd64//conf/GENERIC8 Jul 2023 02:43:02 - 1.518
+++ sys/arch/amd64//conf/GENERIC23 Sep 2023 20:43:00 -
@@ -592,6 +592,7 @@ pgt*at cardbus? # Prism54
Full-MAC
malo* at pci? # Marvell Libertas
malo* at cardbus? # Marvell Libertas
malo* at pcmcia? # Marvell 88W8385
+mbg* at pci? # Meinberg Funkuhren radio clocks
bwfm* at pci? # Broadcom FullMAC
# Media Independent Interface (mii) drivers
Index: sys/dev/pci//mbg.c
===
RCS file: /cvs/src/sys/dev/pci/mbg.c,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 mbg.c
--- sys/dev/pci//mbg.c 10 Apr 2023 04:21:20 - 1.34
+++ sys/dev/pci//mbg.c 23 Sep 2023 20:41:57 -
@@ -160,7 +160,8 @@ const struct pci_matchid mbg_devices[] =
{ PCI_VENDOR_MEINBERG, PCI_PRODUCT_MEINBERG_PCI32 },
{ PCI_VENDOR_MEINBERG, PCI_PRODUCT_MEINBERG_PCI509 },
{ PCI_VENDOR_MEINBERG, PCI_PRODUCT_MEINBERG_PCI511 },
- { PCI_VENDOR_MEINBERG, PCI_PRODUCT_MEINBERG_PEX511 }
+ { PCI_VENDOR_MEINBERG, PCI_PRODUCT_MEINBERG_PEX511 },
+ { PCI_VENDOR_MEINBERG, PCI_PRODUCT_MEINBERG_PZF180PEX }
};
int
@@ -246,6 +247,7 @@ mbg_attach(struct device *parent, struct
sensor_task_register(sc, mbg_task, 10);
break;
case PCI_PRODUCT_MEINBERG_GPS170PCI:
+ case PCI_PRODUCT_MEINBERG_PZF180PEX:
t_trust = 4 * 24 * 60 * 60; /* four days */
sc->sc_read = mbg_read_asic;
sensor_task_register(sc, mbg_task_hr, 1);
Index: sys/dev/pci//pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.2050
diff -u -p -u -p -r1.2050 pcidevs
--- sys/dev/pci//pcidevs7 Sep 2023 02:11:26 - 1.2050
+++ sys/dev/pci//pcidevs23 Sep 2023 20:41:59 -
@@ -7305,6 +7305,7 @@ product MEINBERG PCI320x0101 PCI32
product MEINBERG PCI5090x0102 PCI509
product MEINBERG PCI5110x0104 PCI511
product MEINBERG PEX5110x0105 PEX511
+product MEINBERG PZF180PEX 0x0106 PZF180PEX
product MEINBERG GPS170PCI 0x0204 GPS170PCI
/* Mellanox */
Index: share/man/man4/mbg.4
===
RCS file: /cvs/src/share/man/man4/mbg.4,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 mbg.4
--- share/man/man4/mbg.416 Jul 2013 16:05:49 - 1.14
+++ share/man/man4/mbg.423 Sep 2023 20:41:24 -
@@ -48,6 +48,8 @@ Currently, the following cards are suppo
3.3V/5V DCF77 time signal station receiver card
.It PEX511
PCI Express DCF77 time signal station receiver card
+.It PZF180PEX
+PCI Express DCF77 time signal station receiver card
.El
.Pp
The quality of the timedelta is reported as the sensor status:
OpenBSD 7.4-beta (GENERIC.MP) #0: Sun Sep 24 03:38:15 CEST 2023
maur...@saturn.z74.net:/sys/arch/amd64/compile/GENERIC.MP
real mem = 519962624 (495MB)
avail mem = 484532224 (462MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0
acpi at bios0 not configured
mpbios0 at bios0: Intel MP Specification 1.4
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Atom(TM) CPU E620 @ 600MHz, 600.10 MHz, 06-26-01, patch 0105
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,MOVBE,NXE,LONG,LAHF,PERF,SENSOR,MELTDOWN
cpu0: 24KB 64b/line 6-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line
8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 100MHz
cpu0: mwait min=64, max=64, C-substates=0.2.2.0.2.0.3, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Atom(TM) CPU E620 @ 600MHz, 600.15 MHz, 06-26-01, patch 0105
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAI
Re: Send international text with mail(1) - proposal and patches
Hi Stefan,
Do you like this?
Index: send.c
===
RCS file: /cvs/src/usr.bin/mail/send.c,v
retrieving revision 1.26
diff -u -p -r1.26 send.c
--- send.c 8 Mar 2023 04:43:11 - 1.26
+++ send.c 24 Sep 2023 10:33:11 -
@@ -32,6 +32,11 @@
#include "rcv.h"
#include "extern.h"
+#include "locale.h"
+
+/* To check charset of the message and add the appropiate MIME headers */
+static char nutf8;
+static int not_utf8(FILE *s, int len);
static volatile sig_atomic_t sendsignal; /* Interrupted by a signal? */
@@ -341,6 +346,11 @@ mail1(struct header *hp, int printheader
else
puts("Null message body; hope that's ok");
}
+
+ /* Check non valid UTF-8 characters in the message */
+ nutf8 = not_utf8(mtf, fsize(mtf));
+ rewind(mtf);
+
/*
* Now, take the user names from the combined
* to and cc lists and do all the alias
@@ -520,15 +530,30 @@ puthead(struct header *hp, FILE *fo, int
gotcha = 0;
from = hp->h_from ? hp->h_from : value("from");
if (from != NULL)
- fprintf(fo, "From: %s\n", from), gotcha++;
+ fprintf(fo, "From: %s\n", from),
+ gotcha++;
if (hp->h_to != NULL && w & GTO)
- fmt("To:", hp->h_to, fo, w&GCOMMA), gotcha++;
+ fmt("To:", hp->h_to, fo, w&GCOMMA),
+ gotcha++;
if (hp->h_subject != NULL && w & GSUBJECT)
- fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
+ fprintf(fo, "Subject: %s\n", hp->h_subject),
+ gotcha++;
+ if (nutf8 == 0)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=us-ascii\n"
+ "Content-Transfer-Encoding: 7bit\n"),
+ gotcha++;
+ else if (nutf8 == 1)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=utf-8\n"
+ "Content-Transfer-Encoding: 8bit\n"),
+ gotcha++;
if (hp->h_cc != NULL && w & GCC)
- fmt("Cc:", hp->h_cc, fo, w&GCOMMA), gotcha++;
+ fmt("Cc:", hp->h_cc, fo, w&GCOMMA),
+ gotcha++;
if (hp->h_bcc != NULL && w & GBCC)
- fmt("Bcc:", hp->h_bcc, fo, w&GCOMMA), gotcha++;
+ fmt("Bcc:", hp->h_bcc, fo, w&GCOMMA),
+ gotcha++;
if (gotcha && w & GNL)
(void)putc('\n', fo);
return(0);
@@ -609,4 +634,25 @@ sendint(int s)
{
sendsignal = s;
+}
+
+/* Search non valid UTF-8 characters in the message */
+static int
+not_utf8(FILE *message, int len)
+{
+ int n, ulen;
+ unsigned char s[len];
+ setlocale(LC_CTYPE, "en_US.UTF-8");
+
+ fread(&s, sizeof(char), len, message);
+ ulen = mbstowcs(NULL, s, 0);
+
+ if (ulen == len)
+ n = 0;
+ else if (ulen < 0)
+ n = 2;
+ else if (ulen < len)
+ n = 1;
+
+ return n;
}
--
Walter
Re: Send international text with mail(1) - proposal and patches
On Sun, Sep 24, 2023 at 09:47:41AM +0200, Stefan Sperling wrote: > In the UTF-8 locale I can trigger an error message with your program > by sending the latin1 code for a-acute to stdin. I suppose your test > command didn't actually send latin1 to stdin for some reason? > > $ perl -e 'printf "\xe1rbol\n"' | ./a.out > error: Illegal byte sequence > Right, I can trigger the error with your command, also directly typing the characters in wscons (my keyboard is Spanish), what I was doing in those commands was to copy and paste those latin charactes with my mouse. The strange thing is xterm still showed me the (?) glyphos. Besides, I made a test using the mbstowcs function in my mail patch, and it didn't worked. I'll try again. Thanks Stefan! -- Walter
Re: malloc write after free error checking
On Wed, Sep 20, 2023 at 08:08:23AM +0200, Otto Moerbeek wrote: > On Sun, Sep 03, 2023 at 09:21:18AM +0200, Otto Moerbeek wrote: > > > Hello, > > > > I'm seeing some reports of "write after free" reported by malloc by > > peolpe running current. Malloc has become more strict since begining > > of June. Let me explain: > > > > Small allocations share a page. e.g. a 4k page will hold 8 512 byte > > allocations. > > > > When one such allocation is freed, it will be filled with a "free > > junk" pattern, stay for a short while in the delayed free list (and be > > subhject to write after free checks) and after that it will be marked > > free and it might be allocated again. On a new allocation the write > > after free check will also be done. That is the new part of my commit > > in June. Another change is that on the allocation of the page > > containing chunks, all chunks wil be filled with a "free junk" (0xdf) > > patttern. > > > > The change has a consequence: the time span of the "write after free" > > checks is much longer, as it can take a long time for a chunk to get > > used again. > > > > I do believe the changes itself are correct and can help findiung > > bugs in other code. > > > > You can also be set upon a wrong foot: if an out of bounds write on a > > adjacent chunk happens and lands in (another) free chunk, upon > > allocation of that free chunk it will be reported as a "write after > > free" case. It might even be that an allocation that was never > > allocated and never freed is still "write after free" because of > > "close" out of bounds writes. > > > > I'm thinking how to change the error message to reflect that. > > > > If these extra checks hinder progress, it is possible to swicth them > > off using malloc option f (small F). > > That should be j (small J). > > > For general debugging I recommend using malloc option S, is has all > > the extra strictness that can be enabled while still conforming to the > > malloc API. > > > > Please be aware of these things while hunting for bugs. > > I'm happy to say two of the more complex ones are (being) fixed: one > turned out to be a reference counting bug in firefox. See > http://undeadly.org/cgi?action=article;sid=20230912094727 > > The other, a write after free that crashed the X server when running > picard was diagnosed by me. This one was a bit nasty, as it required > instrumenting malloc to print some extra info to find the root cause. > > The bug is that the call in > https://github.com/openbsd/xenocara/blob/master/xserver/Xext/xvdisp.c#L1002 > overwrites the first 4 bytes of the chunk next to the one allocated on > line 995. > > A workaround is to allocate 4 bytes extra, matthieu@ will be looking > for a proper fix, as it requires knowledge of the X internals. > > I am pondering if the instrumentation hack I used could be modified to > be always available and print the function that did the original > allocation when detecting a write after free. > > The conclusion is that in these two cases, malloc helped in > detecting/finding the bugs (and it was not a bug in malloc itself :-). > Coincidentally this was the topioc of my talk during EuroBSD2023 last > weekend, see http://www.openbsd.org/events.html > > I hope to look at the reports I got for the Wayland port (which is > WIP) as well, see > https://github.com/openbsd/ports/blob/master/wayland/TODO-Wayland.md 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 free)) $ 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). -Otto
Re: Send international text with mail(1) - proposal and patches
On Sun, Sep 24, 2023 at 07:06:35AM +0200, Walter Alejandro Iglesias wrote: > Hi Ingo, > > On Thu, Sep 21, 2023 at 03:04:24PM +0200, Ingo Schwarze wrote: > > In general, the tool for checking the validity of UTF-8 strings > > is a simple loop around mblen(3) if you want to report the precise > > positions of errors found, or simply mbstowcs(3) with a NULL pwcs > > argument if you are content with a one-bit "valid" or "invalid" answer. > > Acording to mbstowcs(3): > > RETURN VALUES > mbstowcs() returns: > > 0 or positive > The value returned is the number of elements stored in the array > pointed to by pwcs, except for a terminating null wide character > (if any). If pwcs is not null and the value returned is equal > to n, the wide-character string pointed to by pwcs is not null > terminated. If pwcs is a null pointer, the value returned is > the number of elements to contain the whole string converted, > except for a terminating null wide character. > > (size_t)-1 The array indirectly pointed to by s contains a byte > sequence forming invalid character. In this case, > mbstowcs() sets errno to indicate the error. > > ERRORS > mbstowcs() may cause an error in the following cases: > > [EILSEQ] s points to the string containing invalid or >incomplete multibyte character. > > > To understand what mbstowcs(3) does I wrote the little test.c program > pasted at bottom. In the following example [a] is UTF-8 aaculte and (a) > iso-latin aacute. > > Using setlocale(LC_CTYPE, "en_US.UTF-8"); > > $ cc -g -Wall test.c > $ echo -n arbol | a.out > ulen: 5 > $ echo -n [a]rbol | a.out > ulen: 5 > $ echo -n (a)rbol | a.out > ulen: 5 In the UTF-8 locale I can trigger an error message with your program by sending the latin1 code for a-acute to stdin. I suppose your test command didn't actually send latin1 to stdin for some reason? $ perl -e 'printf "\xe1rbol\n"' | ./a.out error: Illegal byte sequence > Using setlocale(LC_CTYPE, "C"); > > $ cc -g -Wall test.c > $ echo -n arbol | a.out > ulen: 5 > $ echo -n [a]rbol | a.out > ulen: 6 > $ echo -n (a)rbol | a.out > ulen: 7 > > And no error message in any case. I don't understand in which way those > return values let me know that the third string is invalid UTF-8. Am I > doing something wrong? There is no concept of byte sequences in the C locale, bytes are bytes. It is not possible to detect invalid UTF-8 via libc while running in the C locale since the citrus code in libc won't even run. However, the various ctype tests like isascii(unsigned char)c); isprint((unsigned char)c); and so on can be used to filter or stub out non-ASCII characters, which is what users running in the C locale would want.
