Re: simple pledge for xeyes(1)
On Thu, Sep 07, 2023 at 11:30:11PM -0400, Thomas Frohwein wrote:
> Very basic pledge(2) for the whole program. I didn't dive too much into
> the details and maybe this can be refined some more. This is kind of a
> product of me trying a tool I made `abstain` [1] for usefulness of
> pledge(2) execpromises and it helped quickly find that xeyes(1) can run
> with a very limited set of promises. I tested all permutations of
> running xeyes(1) that are listed in the man page and none of them break
> with this configuration.
>
> ok to add?
>
> [1] https://github.com/rfht/abstain
>
> Index: xeyes.c
> ===
> RCS file: /cvs/xenocara/app/xeyes/xeyes.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 xeyes.c
> --- xeyes.c 29 Aug 2021 17:50:32 - 1.5
> +++ xeyes.c 8 Sep 2023 03:23:51 -
> @@ -38,6 +38,8 @@ from the X Consortium.
> #include "Eyes.h"
> #include
> #include
> +#include
> +#include
> #include "eyes.bit"
> #include "eyesmask.bit"
>
> @@ -111,6 +113,8 @@ main(int argc, char **argv)
> Arg arg[2];
> Cardinal i;
>
> +if(pledge("stdio rpath unix prot_exec", NULL) == -1)
> + err(1, "pledge");
Why is prot_exec needed here?
> XtSetLanguageProc(NULL, (XtLanguageProc) NULL, NULL);
>
> toplevel = XtAppInitialize(_context, "XEyes",
-Bryan.
Re: vmd(8): fix setting log verbosity in child processes
On Wed, Jul 26, 2023 at 12:23:58PM -0400, Dave Voutila wrote:
> When adding exec for vm's and fork/exec'd vio{blk,net} devices, the
> current verbosity wasn't being set on the new process. The below change
> keeps it simple, avoiding runtime string manipulation. Also tosses in an
> ifdef around a very chatty debug message related to ipc with devices.
>
> This doesn't address runtime toggling of verbosity with vmctl(8) nor
> does it address the fact vmd has a janky concept of verbosity. Those are
> future fixes.
>
> ok?
I noticed this earlier, thought I was doing something stupid.
ok brynet@
>
> diffstat /usr/src
> M usr.sbin/vmd/virtio.c | 9+ 4-
> M usr.sbin/vmd/vmd.h | 4+ 0-
> M usr.sbin/vmd/vmm.c | 7+ 4-
>
> 3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff /usr/src
> commit - 3228b0c4b8598ac2f799f997d457a8ba24307bec
> path + /usr/src
> blob - a58e35115432b3d16fb456e71bd71f93d9e2467d
> file + usr.sbin/vmd/virtio.c
> --- usr.sbin/vmd/virtio.c
> +++ usr.sbin/vmd/virtio.c
> @@ -1475,12 +1475,15 @@ virtio_dev_launch(struct vmd_vm *vm, struct virtio_dev
> nargv[5] = "-i";
> nargv[6] = vmm_fd;
> nargv[7] = "-n";
> + nargv[8] = NULL;
>
> - if (env->vmd_verbose) {
> - nargv[8] = "-v";
> + if (env->vmd_verbose == 1) {
> + nargv[8] = VMD_VERBOSE_1;
> nargv[9] = NULL;
> - } else
> - nargv[8] = NULL;
> + } else if (env->vmd_verbose > 1) {
> + nargv[8] = VMD_VERBOSE_2;
> + nargv[9] = NULL;
> + }
>
> /* Control resumes in vmd.c:main(). */
> execvp(nargv[0], nargv);
> @@ -1699,8 +1702,10 @@ virtio_pci_io(int dir, uint16_t reg, uint32_t *data, u
> imsg_free();
>
> if (msg.type == VIODEV_MSG_IO_READ && msg.data_valid) {
> +#if DEBUG
> log_debug("%s: got sync read response (reg=%s)",
> __func__, virtio_reg_name(msg.reg));
> +#endif /* DEBUG */
> *data = msg.data;
> /*
>* It's possible we're asked to {de,}assert after the
> blob - 744b8d1957423b91202b9630fe4a5a6dc4158089
> file + usr.sbin/vmd/vmd.h
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -102,6 +102,10 @@ enum imsg_type {
> /* Unique local address for IPv6 */
> #define VMD_ULA_PREFIX "fd00::/8"
>
> +/* Verbosity arguments for use when caling execvp(2). */
> +#define VMD_VERBOSE_1"-v";
> +#define VMD_VERBOSE_2"-vv";
> +
> enum imsg_type {
> IMSG_VMDOP_START_VM_REQUEST = IMSG_PROC_MAX,
> IMSG_VMDOP_START_VM_CDROM,
> blob - 541222e027294ea6d85c957e9cc1a55bb1ac829c
> file + usr.sbin/vmd/vmm.c
> --- usr.sbin/vmd/vmm.c
> +++ usr.sbin/vmd/vmm.c
> @@ -782,12 +782,15 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *p
> nargv[3] = "-n";
> nargv[4] = "-i";
> nargv[5] = vmm_fd;
> + nargv[6] = NULL;
>
> - if (env->vmd_verbose) {
> - nargv[6] = "-v";
> + if (env->vmd_verbose == 1) {
> + nargv[6] = VMD_VERBOSE_1;
> nargv[7] = NULL;
> - } else
> - nargv[6] = NULL;
> + } else if (env->vmd_verbose > 1) {
> + nargv[6] = VMD_VERBOSE_2;
> + nargv[7] = NULL;
> + }
>
> /* Control resumes in vmd main(). */
> execvp(nargv[0], nargv);
>
>
Re: cwm: add fvwm and tvm as default wm entries
On Mon, May 15, 2023 at 09:17:00AM -0400, Okan Demirmen wrote: > On Mon 2023.05.15 at 10:41 +0200, Matthieu Herrb wrote: > > On Mon, May 15, 2023 at 06:26:41AM +, Klemens Nanni wrote: > > > Both fvwm(1) and twm(1) have a restart menu that contains other window > > > managers by default, which is useful if you want to switch around > > > without restarting X and/or custom window manager config. > > > > > > cwm(1) only offers to restart into itself by deafult. > > > Add the other two we ship by default so users can round trip between > > > them. > > > > > > Feedback? OK? > > > > Last year I mentionned that I think we should retire twm. It's really > > too old and missing support for the modern window managers hints. > > > > People still using it should switch to cwm or maybe ctwm from ports > > (to keep the same configurarion system), or someone should step up to > > maintain it and enhance it with exwmh support. (but this is somehow > > just wasting time imho). > > > > Otherwise ok to add this and fix the other WM menus for other window > > managers (those parts of the configs are already local changes in > > Xenocara) > > I might argue the opposite, to remove cwm from fvwm and twm restart menus, if > this inconsistency is a real concern. The entries in fvwm/twm are in the > (shipped) example config files, where-as below it is, well, there for good > with > no user choice. Heck, how often to do people even use this restart wm to > another WM outside of playing around? Most window managers handle restarts > differently, regardless of what ICCCM/EWMH says) and even then, crossing > window > managers like this introduces inconsistencies. It's fine for playing around I > suppose, but is it really a demanded "workflow"? > > > > > > > PS: fwvm and twm menus more programs we don't ship, e.g. "wm2", and > > > twm dies when failing to execute them (fvwm and cwm keeps running); > > > do we want to keep those default-broken entries around? > > I'd support removing them. +1 I don't think hardcoding window managers into cwm makes sense. I don't think anyone is actually switching around WMs at runtime like this. If the point is for new users to have an example that provides a list of alternative WMs, perhaps this is a man page issue and they should be added to "SEE ALSO" sections. -Bryan.
Re: AMD 17h/1xh HD Audio testers wanted!
On Sun, Mar 05, 2023 at 08:53:00AM +0100, Alexandre Ratchov wrote:
> If you've an azalia(4) attaching as "AMD 17h/1xh HD Audio", please
> test this diff and report regressions. Especially audio lock ups that
> require reboot.
>
> IIRC, MSI was disabled few years ago to "fix" such lockups, and now
> this diff suggests we need MSI on certain boards.
>
> Context and diff below:
>
> - Forwarded message from Andreas Bartelt -
>
> Date: Sat, 4 Mar 2023 16:12:22 +0100
> From: Andreas Bartelt
> To: Alexandre Ratchov , b...@openbsd.org
> Subject: Re: audio(4) output doesn't work yet on ASUS ProArt X670E-CREATOR
> WIFI mainboard (ALC1220
> CODEC)
> User-Agent: Mozilla/5.0 (X11; OpenBSD amd64; rv:102.0) Gecko/20100101
> Thunderbird/102.8.0
>
> On 2/27/23 6:41 PM, Andreas Bartelt wrote:
> > On 2/27/23 2:40 PM, Alexandre Ratchov wrote:
> > > On Sat, Feb 25, 2023 at 05:20:53PM +0100, Andreas Bartelt wrote:
> > > > Hi,
> > > >
> > > > I've tested a recent OpenBSD snapshot of CURRENT on an ASUS ProArt
> > > > X670E-CREATOR WIFI mainboard. According to the information
> > > > provided by ASUS,
> > > > this mainboard features a "Realtek S1220A CODEC" which attaches as
> > > > Realtek
> > > > ALC1220 on OpenBSD -- however, audio output (tested with
> > > > headphones on the
> > > > line out connector) doesn't work there yet. Applications (e.g., mplayer,
> > > > mpg123) hang and I can hear no sound.
> > > >
> > > > [I don't know if this helps but I previously also had access to an
> > > > ASUS ROG
> > > > STRIX B550-E GAMING mainboard which, according to ASUS, also features an
> > > > S1220A CODEC which also attaches as Realtek ALC1220 on OpenBSD -- audio
> > > > output (tested on the line out connector) works there without problems.]
> > > >
> > > > In order to verify that the new mainboard doesn't have a physical defect
> > > > with regard to the line out audio connector, I've also tested a
> > > > FreeBSD 13.2
> > > > BETA3 snapshot on the ASUS ProArt X670E-CREATOR WIFI mainboard.
> > > > Audio output
> > > > worked there out-of-the-box, so this might be a fixable problem on
> > > > OpenBSD.
> > > >
> > > > I've found some info with regard to audio debugging at
> > > > https://www.openbsd.org/faq/faq13.html#audioprob . While running
> > > > # cat > /dev/audio0 < /dev/zero
> > > > play.bytes doesn't increase at all:
> > > > # audioctl play.{bytes,errors}
> > > > play.bytes=0
> > > > play.errors=0
> > > >
> > >
> > > mixerctl shows that the host manages communicate with the codec, but
> > > above lines suggest that DMA doesn't start. Could you check if there
> > > are any audio-related options in the BIOS? Especially, if there's an
> > > option to disable the microphone (or "recording" or alike), please
> > > enable it.
> >
> > There's no microphone or recording specific options available. I could
> > only identify a single audio related configuration option. Under
> > Advanced\Onboard Devices Configuration: enable/disable "HD Audio
> > Controller" (description says Enable/Disable Azalia HD Audio). It does
> > exactly that, i.e., disabling this option removes the azalia1 device from
> > OpenBSD's dmesg.
> >
> > With this option enabled again, mp3 playback works with FreeBSD but hangs
> > with OpenBSD -- same BIOS config.
> >
>
> I've made audio work on the ASUS ProArt X670E-CREATOR WIFI mainboard, simply
> by enabling msi.
>
> azalia1 at pci21 dev 0 function 6 "AMD 17h/1xh HD Audio" rev 0x00: msi
> azalia1: codecs: Realtek ALC1220
> audio0 at azalia1
>
> The following diff fixes the problem:
> Index: src/sys/dev/pci/azalia.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> retrieving revision 1.283
> diff -u -p -r1.283 azalia.c
> --- src/sys/dev/pci/azalia.c 21 Feb 2023 13:42:59 - 1.283
> +++ src/sys/dev/pci/azalia.c 4 Mar 2023 15:02:31 -
> @@ -554,7 +554,6 @@ azalia_pci_attach(struct device *parent,
> if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_AMD) {
> switch (PCI_PRODUCT(sc->pciid)) {
> case PCI_PRODUCT_AMD_17_HDA:
> - case PCI_PRODUCT_AMD_17_1X_HDA:
> case PCI_PRODUCT_AMD_HUDSON2_HDA:
> pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
> }
>
> OK?
>
>
> - End forwarded message -
No, this workaround is still needed. thfr@ and I tried to debug this years
ago but could not determine the cause at the time.
This audio hang is still there on many systems, e.g: playback works for a
breif time until the it hangs, and only a reboot will fix it. But we could
never reproduce it with MSI disabled, so that was the best option we had.
-Bryan.
Re: mem.4: be more accurate about securelevel
On Tue, Jan 17, 2023 at 09:37:24PM +0100, Jan Klemkow wrote:
> Hi,
>
> This diff adjust the manpage of mem(4) to be more accurate. You can
> open(2) mem(4) in securelevel 1 in readonly mode, but not writable.
>
> kern/spec_vnops.c:
>
> if (ap->a_cred != FSCRED && (ap->a_mode & FWRITE)) {
> ...
> /*
>* When running in secure mode, do not allow opens
>* for writing of /dev/mem, /dev/kmem, or character
>* devices whose corresponding block devices are
>* currently mounted.
>*/
> if (securelevel >= 1) {
> ...
> if (iskmemdev(dev))
> return (EPERM);
> }
> }
>
> OK?
>
> bye,
> Jan
Are you sure about that? Have you tested it?
https://github.com/openbsd/src/commit/19aedf236181e81baf170421900911c82671fae4
> Index: man4.alpha/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.alpha/mem.4,v
> retrieving revision 1.6
> diff -u -p -r1.6 mem.4
> --- man4.alpha/mem.4 12 Jan 2018 04:36:44 - 1.6
> +++ man4.alpha/mem.4 17 Jan 2023 18:51:10 -
> @@ -62,7 +62,7 @@ kernel virtual memory begins at
> .Li 0xfc23 .
> .Pp
> Even with sufficient file system permissions,
> -these devices can only be opened when the
> +these devices can only be opened writable when the
> .Xr securelevel 7
> is insecure or when the
> .Va kern.allowkmem
> Index: man4.amd64/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.amd64/mem.4,v
> retrieving revision 1.6
> diff -u -p -r1.6 mem.4
> --- man4.amd64/mem.4 12 Jan 2018 04:36:44 - 1.6
> +++ man4.amd64/mem.4 17 Jan 2023 18:48:23 -
> @@ -63,7 +63,7 @@ The kernel virtual memory begins at addr
> .Li 0x8000 .
> .Pp
> Even with sufficient file system permissions,
> -these devices can only be opened when the
> +these devices can only be opened writable when the
> .Xr securelevel 7
> is insecure or when the
> .Va kern.allowkmem
> Index: man4.hppa/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.hppa/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.hppa/mem.4 12 Jan 2018 04:36:44 - 1.4
> +++ man4.hppa/mem.4 17 Jan 2023 18:52:28 -
> @@ -51,7 +51,7 @@ On hppa, the physical memory range is al
> address 0; kernel virtual memory begins at address 0 as well.
> .Pp
> Even with sufficient file system permissions,
> -these devices can only be opened when the
> +these devices can only be opened writable when the
> .Xr securelevel 7
> is insecure or when the
> .Va kern.allowkmem
> Index: man4.i386/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.i386/mem.4,v
> retrieving revision 1.12
> diff -u -p -r1.12 mem.4
> --- man4.i386/mem.4 12 Jan 2018 04:36:44 - 1.12
> +++ man4.i386/mem.4 17 Jan 2023 18:53:00 -
> @@ -63,7 +63,7 @@ long, and ends at virtual address
> .Li 0xfe00 .
> .Pp
> Even with sufficient file system permissions,
> -these devices can only be opened when the
> +these devices can only be opened writable when the
> .Xr securelevel 7
> is insecure or when the
> .Va kern.allowkmem
> Index: man4.landisk/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.landisk/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.landisk/mem.412 Jan 2018 04:36:44 - 1.4
> +++ man4.landisk/mem.417 Jan 2023 18:53:54 -
> @@ -58,7 +58,7 @@ The kernel virtual memory begins at addr
> .Li 0xc000 .
> .Pp
> Even with sufficient file system permissions,
> -these devices can only be opened when the
> +these devices can only be opened writable when the
> .Xr securelevel 7
> is insecure or when the
> .Va kern.allowkmem
> Index: man4.loongson/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.loongson/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.loongson/mem.4 12 Jan 2018 04:36:44 - 1.4
> +++ man4.loongson/mem.4 17 Jan 2023 18:54:33 -
> @@ -88,7 +88,7 @@ The kernel virtual memory begins at addr
> .Ad 0xc000 .
> .Pp
> Even with sufficient file system permissions,
> -these devices can only be opened when the
> +these devices can only be opened writable when the
> .Xr securelevel 7
> is insecure or when the
> .Va kern.allowkmem
> Index: man4.luna88k/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.luna88k/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.luna88k/mem.412 Jan 2018
Re: remove games from PATHs provided by /etc/skel
On Thu, Aug 04, 2022 at 08:39:46PM -0600, Theo de Raadt wrote: > Bryan Steele wrote: > > > On Fri, Aug 05, 2022 at 03:37:41AM +0200, Theo Buehler wrote: > > > On Fri, Aug 05, 2022 at 03:34:57AM +0200, Theo Buehler wrote: > > > > If you want games, opt into it. They are very old, full of bugs and not > > > > really maintained. It's very easy to get a PATH containing games via > > > > /etc/skel. I think this is a poor default. > > > > > > Dropped a } by accident. > > > > If they are really unmaintained, then they should be removed from the > > tree, no? Otherwise I don't really see the point in this. > > Want to talk about something that is unmaintained? > > vi. > > Sorry, but I don't think you are in a position to push policy. I responded too quickly, sorry. It was less so my intention to suggest that thing be removed, but the opposite. Sorry for the noise..
Re: remove games from PATHs provided by /etc/skel
On Fri, Aug 05, 2022 at 03:37:41AM +0200, Theo Buehler wrote:
> On Fri, Aug 05, 2022 at 03:34:57AM +0200, Theo Buehler wrote:
> > If you want games, opt into it. They are very old, full of bugs and not
> > really maintained. It's very easy to get a PATH containing games via
> > /etc/skel. I think this is a poor default.
>
> Dropped a } by accident.
If they are really unmaintained, then they should be removed from the
tree, no? Otherwise I don't really see the point in this.
-Bryan.
> Index: dot.cshrc
> ===
> RCS file: /cvs/src/etc/skel/dot.cshrc,v
> retrieving revision 1.10
> diff -u -p -r1.10 dot.cshrc
> --- dot.cshrc 24 Jan 2020 02:09:51 - 1.10
> +++ dot.cshrc 5 Aug 2022 01:37:02 -
> @@ -13,7 +13,7 @@ alias llls -lsA
> alias tset 'set noglob histchars=""; eval `\tset -s \!*`; unset noglob
> histchars'
> alias z suspend
>
> -set path = (~/bin /bin /sbin
> /usr/{bin,sbin,X11R6/bin,local/bin,local/sbin,games})
> +set path = (~/bin /bin /sbin /usr/{bin,sbin,X11R6/bin,local/bin,local/sbin})
>
> if ($?prompt) then
> # An interactive shell -- set some stuff up
> Index: dot.profile
> ===
> RCS file: /cvs/src/etc/skel/dot.profile,v
> retrieving revision 1.7
> diff -u -p -r1.7 dot.profile
> --- dot.profile 24 Jan 2020 02:09:51 - 1.7
> +++ dot.profile 5 Aug 2022 01:30:52 -
> @@ -2,5 +2,5 @@
> #
> # sh/ksh initialization
>
> -PATH=$HOME/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games
> +PATH=$HOME/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin
> export PATH HOME TERM
>
>
Re: rpki-client unveil main process
On Thu, Aug 04, 2022 at 12:47:36PM +0100, Ricardo Mestre wrote:
> We are using pledge so if you don't remove the unveil permission it will be
> allowed throughtout the entire process, so please just change unveil(NULL,
> NULL)
> to old previous pledge("stdio rpath wpath cpath fattr sendfd").
>
> Thank you :)
Stylistically I agree.
It's equivalent. unveil(2) will return EPERM once locked, even if the
process hasn't dropped the unveil promise.
> On 12:29 Thu 04 Aug , Claudio Jeker wrote:
> > On Thu, Aug 04, 2022 at 12:24:03PM +0200, Theo Buehler wrote:
> > > On Thu, Aug 04, 2022 at 12:11:45PM +0200, Claudio Jeker wrote:
> > > > This diff adds unveil to the main process. This is done after all files
> > > > from the command line have been read. Both for regular and -f mode.
> > > > Once the args have been read the process can limit the access to the
> > > > cachedir and the output dir. In -f mode only read access to the cachdir
> > > > is
> > > > required. In regular both cachedir and outputdir need rwc rights.
> > >
> > >
> > > >
> > > > --
> > > > :wq Claudio
> > > >
> > > > Index: main.c
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> > > > retrieving revision 1.208
> > > > diff -u -p -r1.208 main.c
> > > > --- main.c 27 Jun 2022 10:18:27 - 1.208
> > > > +++ main.c 28 Jul 2022 16:57:16 -
> > > > @@ -1006,8 +1006,7 @@ main(int argc, char *argv[])
> > > > signal(SIGALRM, suicide);
> > > > }
> > > >
> > > > - /* TODO unveil cachedir and outputdir, no other access allowed
> > > > */
> > > > - if (pledge("stdio rpath wpath cpath fattr sendfd", NULL) == -1)
> > > > + if (pledge("stdio rpath wpath cpath fattr sendfd unveil", NULL)
> > > > == -1)
> > > > err(1, "pledge");
> > > >
> > > > msgbuf_init();
> > > > @@ -1049,6 +1048,18 @@ main(int argc, char *argv[])
> > > > while (*argv != NULL)
> > > > queue_add_file(*argv++, RTYPE_FILE, 0);
> > > > }
> > >
> > > This brace ends an if (filemode) block. I'm wondering if this would not
> > > be cleaner:
> > >
> > > if (filemode) {
> > > while (*argv != NULL)
> > > queue_add_file(*argv++, RTYPE_FILE, 0);
> > >
> > > if (unveil(cachedir, "r") == -1)
> > > err(1, "unveil cachedir");
> > > } else {
> > > if (unveil(outputdir, "rwc") == -1)
> > > err(1, "unveil outputdir");
> > > if (unveil(cachedir, "rwc") == -1)
> > > err(1, "unveil cachedir");
> > > }
> > > if (unveil(NULL, NULL) == -1)
> > > err(1, "unveil");
> > >
> > > Either way ok
> >
> > Sure, good suggestion. Will commit that version.
> >
> > --
> > :wq Claudio
> >
>
>
Re: acpitz(4): perform passive cooling only when perfpolicy is AUTO
On Mon, Jun 27, 2022 at 11:01:31PM +0200, Stefan Hagen wrote:
> Hi,
>
> acpitz(4) implements passive cooling, which starts throttling the CPU to
> keep it under the temperature reported by the _PSV trip point.
>
> https://uefi.org/specs/ACPI/6.4/11_Thermal_Management/thermal-control.html
>
> The specs (1.1.5.1) leave the decision to activate passive cooling to
> the OS. It is a way to limit noise and heat rather than to protect the
> CPU (for which _HOT and _CRT are the better trip points).
>
> I would like to restrict passive cooling to the AUTO perfpolicy.
>
> For low (apm -L) and high (apm -H) it doesn't make much sense, because
> - low is setting a low pstate anyway
> - high is probably never set with the intention to have a cool and
>quiet machine.
>
> Compiling a kernel with apm -H before diff:
> 4m52.32s real21m02.62s user 4m47.00s system
>
> Compiling a kernel with apm -H after diff:
> 2m42.65s real11m36.07s user 2m36.83s system
>
> OK? Comments?
>
> Best Regards,
> Stefan
Shouldn't this also take into consideration hw.power as well? If it
doesn't make sense for perfpolicy=high then it probably doesn't for
perfpolicy=auto when on AC power?
>
> Index: sys/dev/acpi/acpitz.c
> ===
> RCS file: /home/cvs/src/sys/dev/acpi/acpitz.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 acpitz.c
> --- sys/dev/acpi/acpitz.c 6 Apr 2022 18:59:27 - 1.58
> +++ sys/dev/acpi/acpitz.c 27 Jun 2022 19:25:55 -
> @@ -90,6 +90,7 @@ void(*acpitz_cpu_setperf)(int);
> int acpitz_perflevel = -1;
> extern void (*cpu_setperf)(int);
> extern int perflevel;
> +extern int perfpolicy;
> #define PERFSTEP 10
>
> #define ACPITZ_TRIPS (1L << 0)
> @@ -381,7 +382,7 @@ acpitz_refresh(void *arg)
> sc->sc_tc1, sc->sc_tc2, sc->sc_psv);
>
> nperf = acpitz_perflevel;
> - if (sc->sc_psv <= sc->sc_tmp) {
> + if (sc->sc_psv <= sc->sc_tmp && perfpolicy == 1) {
> /* Passive cooling enabled */
> dnprintf(1, "%s: enabling passive %d %d\n",
> DEVNAME(sc), sc->sc_tmp, sc->sc_psv);
>
>
Re: Wayland Display server
On Tue, May 24, 2022 at 01:18:20AM +0200, Daniel Douglas Dyrseth wrote: > I know a little C, but not enough to get any profound progress on making a > Wayland WM for OpenBSD. Not to be rude, but could someone that has a coding > team for OpenBSD make one? It will shrink the OS a lot and optimize and > secure further when stable. > > Sincerely > Daniel Douglas Dyrseth This is off-topic for tech@, >From https://www.openbsd.org/mail.html Discussion of technical topics for OpenBSD developers and advanced users. This is _not_ a "tech support" forum - do not use it as such.
Re: clang-local.1: document support for source-based code coverage
On Wed, May 04, 2022 at 05:40:43PM +0200, Marc Espie wrote: > On Wed, May 04, 2022 at 07:43:35AM -0400, Bryan Steele wrote: > > On Wed, May 04, 2022 at 01:20:10PM +0200, Frederic Cambus wrote: > > > Hi tech@, > > > > > > The base system includes the compiler-rt profile library for > > > source-based code coverage. > > > > > > So here is a diff to document support in clang-local.1, the same > > > way we document support for the ubsan_minimal sanitizer runtime > > > which is also part of compiler-rt. > > > > > > Comments? OK? > > > > > > Index: share/man/man1/clang-local.1 > > > === > > > RCS file: /cvs/src/share/man/man1/clang-local.1,v > > > retrieving revision 1.23 > > > diff -u -p -r1.23 clang-local.1 > > > --- share/man/man1/clang-local.1 18 Feb 2022 00:39:18 - 1.23 > > > +++ share/man/man1/clang-local.1 4 May 2022 11:03:11 - > > > @@ -99,6 +99,15 @@ See the documentation for the > > > .Fl fsanitize-minimal-runtime > > > flag. > > > .It > > > +The base system includes the compiler-rt profile library for > > > +source-based code coverage. See the documentation for the > > > +.Fl fprofile-instr-generate > > > +and > > > +.Fl fcoverage-mapping > > > +flags. > > > +Note that llvm-profdata and llvm-cov tools from devel/llvm are > > > +required to process coverage data and produce reports. > > > +.It > > > The > > > .Xr malloc 3 , > > > .Xr calloc 3 , > > > > Isn't the purpose of the clang-local(1) to document local changes to the > > system compiler, -fsanitize-minimal-runtime feels like a special case as > > we do not have the complete sanitizer runtime. > > What do you suggest as a good location where people will > find that information easily ? As unfortunate as it is, clang documention is mostly on the website[0]. I don't see why this needs to be in clang-local, is there some reason why users would expect this to not work by default on OpenBSD? -Bryan. [0] https://releases.llvm.org/13.0.0/tools/clang/docs/UsersManual.html
Re: clang-local.1: document support for source-based code coverage
On Wed, May 04, 2022 at 01:20:10PM +0200, Frederic Cambus wrote: > Hi tech@, > > The base system includes the compiler-rt profile library for > source-based code coverage. > > So here is a diff to document support in clang-local.1, the same > way we document support for the ubsan_minimal sanitizer runtime > which is also part of compiler-rt. > > Comments? OK? > > Index: share/man/man1/clang-local.1 > === > RCS file: /cvs/src/share/man/man1/clang-local.1,v > retrieving revision 1.23 > diff -u -p -r1.23 clang-local.1 > --- share/man/man1/clang-local.1 18 Feb 2022 00:39:18 - 1.23 > +++ share/man/man1/clang-local.1 4 May 2022 11:03:11 - > @@ -99,6 +99,15 @@ See the documentation for the > .Fl fsanitize-minimal-runtime > flag. > .It > +The base system includes the compiler-rt profile library for > +source-based code coverage. See the documentation for the > +.Fl fprofile-instr-generate > +and > +.Fl fcoverage-mapping > +flags. > +Note that llvm-profdata and llvm-cov tools from devel/llvm are > +required to process coverage data and produce reports. > +.It > The > .Xr malloc 3 , > .Xr calloc 3 , Isn't the purpose of the clang-local(1) to document local changes to the system compiler, -fsanitize-minimal-runtime feels like a special case as we do not have the complete sanitizer runtime. -Bryan.
Re: beef up ksmn(4) to show more temps and CPU frequency
On Mon, Apr 25, 2022 at 05:33:51PM +0200, Claudio Jeker wrote: > On Mon, Apr 25, 2022 at 11:31:22AM -0400, Bryan Steele wrote: > > On Mon, Apr 25, 2022 at 05:20:46PM +0200, Claudio Jeker wrote: > > > On Sun, Apr 24, 2022 at 07:06:19PM +0200, Claudio Jeker wrote: > > > > On Ryzen CPUs each CCD has a temp sensor. If the CPU has CCDs (which > > > > excludes Zen APU CPUs) this should show additional temp info. This is > > > > based on info from the Linux k10temp driver. > > > > > > > > Additionally use the MSRs defined in "Open-Source Register Reference For > > > > AMD Family 17h Processors" to measure the CPU core frequency. > > > > That should be the actuall speed of the CPU core during the measuring > > > > interval. > > > > > > > > On my T14g2 the output is now for example: > > > > ksmn0.temp0 63.88 degC Tctl > > > > ksmn0.frequency03553141515.00 Hz CPU0 > > > > ksmn0.frequency13549080315.00 Hz CPU2 > > > > ksmn0.frequency23552369937.00 Hz CPU4 > > > > ksmn0.frequency33546055048.00 Hz CPU6 > > > > ksmn0.frequency43546854449.00 Hz CPU8 > > > > ksmn0.frequency53543869698.00 Hz CPU10 > > > > ksmn0.frequency63542551127.00 Hz CPU12 > > > > ksmn0.frequency74441623647.00 Hz CPU14 > > > > > > > > It is intresting to watch turbo kick in and how temp causes the CPU to > > > > throttle. > > > > > > > > I only tested this on systems with APUs so I could not thest the Tccd > > > > temp > > > > reporting. > > > > > > With the frequence sensor moved to cpu(4) this just adds the Tccd > > > additional temparature sensors. It does not fix the duplication of the > > > ksmn(4) sensors. That needs to be fixed by no attaching on the duplicate > > > root complexes. How that is done I still need to figure out. > > > > > > -- > > > :wq Claudio > > > > I think this looks good now, I'm not too worried about the tCTL offsets. > > That can be figured out later. ok brynet@ > > > > As for the attaching issue, would only attaching one device in the > > kernel device be a nasty hack? If I understand, this should make the > > duplicate complex attach as ppb(4) again. > > > > ksmn0 at pci? > > > > But that would break systems with multiple CPU sockets. Like the two > socket EPYC server. Ah, I missed that possibility. > -- > :wq Claudio
Re: beef up ksmn(4) to show more temps and CPU frequency
On Mon, Apr 25, 2022 at 05:20:46PM +0200, Claudio Jeker wrote:
> On Sun, Apr 24, 2022 at 07:06:19PM +0200, Claudio Jeker wrote:
> > On Ryzen CPUs each CCD has a temp sensor. If the CPU has CCDs (which
> > excludes Zen APU CPUs) this should show additional temp info. This is
> > based on info from the Linux k10temp driver.
> >
> > Additionally use the MSRs defined in "Open-Source Register Reference For
> > AMD Family 17h Processors" to measure the CPU core frequency.
> > That should be the actuall speed of the CPU core during the measuring
> > interval.
> >
> > On my T14g2 the output is now for example:
> > ksmn0.temp0 63.88 degC Tctl
> > ksmn0.frequency03553141515.00 Hz CPU0
> > ksmn0.frequency13549080315.00 Hz CPU2
> > ksmn0.frequency23552369937.00 Hz CPU4
> > ksmn0.frequency33546055048.00 Hz CPU6
> > ksmn0.frequency43546854449.00 Hz CPU8
> > ksmn0.frequency53543869698.00 Hz CPU10
> > ksmn0.frequency63542551127.00 Hz CPU12
> > ksmn0.frequency74441623647.00 Hz CPU14
> >
> > It is intresting to watch turbo kick in and how temp causes the CPU to
> > throttle.
> >
> > I only tested this on systems with APUs so I could not thest the Tccd temp
> > reporting.
>
> With the frequence sensor moved to cpu(4) this just adds the Tccd
> additional temparature sensors. It does not fix the duplication of the
> ksmn(4) sensors. That needs to be fixed by no attaching on the duplicate
> root complexes. How that is done I still need to figure out.
>
> --
> :wq Claudio
I think this looks good now, I'm not too worried about the tCTL offsets.
That can be figured out later. ok brynet@
As for the attaching issue, would only attaching one device in the
kernel device be a nasty hack? If I understand, this should make the
duplicate complex attach as ppb(4) again.
ksmn0 at pci?
> Index: ksmn.c
> ===
> RCS file: /cvs/src/sys/dev/pci/ksmn.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 ksmn.c
> --- ksmn.c11 Mar 2022 18:00:50 - 1.6
> +++ ksmn.c25 Apr 2022 15:19:21 -
> @@ -42,6 +42,7 @@
>* [31:21] Current reported temperature.
>*/
> #define SMU_17H_THM 0x59800
> +#define SMU_17H_CCD_THM(o, x)(SMU_17H_THM + (o) + ((x) * 4))
> #define GET_CURTMP(r)(((r) >> 21) & 0x7ff)
>
> /*
> @@ -50,6 +51,8 @@
> */
> #define CURTMP_17H_RANGE_SEL (1 << 19)
> #define CURTMP_17H_RANGE_ADJUST 490
> +#define CURTMP_CCD_VALID (1 << 11)
> +#define CURTMP_CCD_MASK 0x7ff
>
> /*
> * Undocumented tCTL offsets gleamed from Linux k10temp driver.
> @@ -75,13 +78,18 @@ struct ksmn_softc {
> pcitag_tsc_pcitag;
>
> int sc_tctl_offset;
> + unsigned intsc_ccd_valid; /* available Tccds */
> + unsigned intsc_ccd_offset;
>
> - struct ksensor sc_sensor;
> struct ksensordev sc_sensordev;
> + struct ksensor sc_sensor; /* Tctl */
> + struct ksensor sc_ccd_sensor[12]; /* Tccd */
> };
>
> int ksmn_match(struct device *, void *, void *);
> void ksmn_attach(struct device *, struct device *, void *);
> +uint32_t ksmn_read_reg(struct ksmn_softc *, uint32_t);
> +void ksmn_ccd_attach(struct ksmn_softc *, int);
> void ksmn_refresh(void *);
>
> const struct cfattach ksmn_ca = {
> @@ -113,7 +121,9 @@ ksmn_attach(struct device *parent, struc
> struct ksmn_softc *sc = (struct ksmn_softc *)self;
> struct pci_attach_args *pa = aux;
> struct curtmp_offset*p;
> - extern char cpu_model[];
> + struct cpu_info *ci = curcpu();
> + extern char cpu_model[];
> +
>
> sc->sc_pc = pa->pa_pc;
> sc->sc_pcitag = pa->pa_tag;
> @@ -122,6 +132,7 @@ ksmn_attach(struct device *parent, struc
> sizeof(sc->sc_sensordev.xname));
>
> sc->sc_sensor.type = SENSOR_TEMP;
> + snprintf(sc->sc_sensor.desc, sizeof(sc->sc_sensor.desc), "Tctl");
> sensor_attach(>sc_sensordev, >sc_sensor);
>
> /*
> @@ -136,6 +147,38 @@ ksmn_attach(struct device *parent, struc
> sc->sc_tctl_offset = p->tctl_offset;
> }
>
> + sc->sc_ccd_offset = 0x154;
> +
> + if (ci->ci_family == 0x17 || ci->ci_family == 0x18) {
> + switch (ci->ci_model) {
> + case 0x1: /* Zen */
> + case 0x8: /* Zen+ */
> + case 0x11: /* Zen APU */
> + case 0x18: /* Zen+ APU */
> + ksmn_ccd_attach(sc, 4);
> + break;
> + case 0x31: /* Zen2 Threadripper */
> + case 0x60: /* Renoir */
> + case 0x68: /* Lucienne */
> +
Re: beef up ksmn(4) to show more temps and CPU frequency
On Sun, Apr 24, 2022 at 07:06:19PM +0200, Claudio Jeker wrote:
> On Ryzen CPUs each CCD has a temp sensor. If the CPU has CCDs (which
> excludes Zen APU CPUs) this should show additional temp info. This is
> based on info from the Linux k10temp driver.
>
> Additionally use the MSRs defined in "Open-Source Register Reference For
> AMD Family 17h Processors" to measure the CPU core frequency.
> That should be the actuall speed of the CPU core during the measuring
> interval.
>
> On my T14g2 the output is now for example:
> ksmn0.temp0 63.88 degC Tctl
> ksmn0.frequency03553141515.00 Hz CPU0
> ksmn0.frequency13549080315.00 Hz CPU2
> ksmn0.frequency23552369937.00 Hz CPU4
> ksmn0.frequency33546055048.00 Hz CPU6
> ksmn0.frequency43546854449.00 Hz CPU8
> ksmn0.frequency53543869698.00 Hz CPU10
> ksmn0.frequency63542551127.00 Hz CPU12
> ksmn0.frequency74441623647.00 Hz CPU14
>
> It is intresting to watch turbo kick in and how temp causes the CPU to
> throttle.
>
> I only tested this on systems with APUs so I could not thest the Tccd temp
> reporting.
> --
> :wq Claudio
Awesome! :-)
I can see this adding a bunch of extra frequency sensors on higher-end
Ryzen/Threadripper/EPYC CPUs, considering it's displayed in "Hz" that
might be a bit overwhelming to look at.. but that's just a cosmetic
nit for systat.
I'll test on some of my AMD machines later, if someone doesn't beat me
to it.
Some comments below..
> Index: ksmn.c
> ===
> RCS file: /cvs/src/sys/dev/pci/ksmn.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 ksmn.c
> --- ksmn.c11 Mar 2022 18:00:50 - 1.6
> +++ ksmn.c24 Apr 2022 16:47:08 -
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -42,6 +43,7 @@
>* [31:21] Current reported temperature.
>*/
> #define SMU_17H_THM 0x59800
> +#define SMU_17H_CCD_THM(o, x)(SMU_17H_THM + (o) + ((x) * 4))
> #define GET_CURTMP(r)(((r) >> 21) & 0x7ff)
>
> /*
> @@ -50,6 +52,8 @@
> */
> #define CURTMP_17H_RANGE_SEL (1 << 19)
> #define CURTMP_17H_RANGE_ADJUST 490
> +#define CURTMP_CCD_VALID (1 << 11)
> +#define CURTMP_CCD_MASK 0x7ff
>
> /*
> * Undocumented tCTL offsets gleamed from Linux k10temp driver.
> @@ -75,13 +79,24 @@ struct ksmn_softc {
> pcitag_tsc_pcitag;
>
> int sc_tctl_offset;
> + unsigned intsc_ccd_valid; /* available Tccds */
> + unsigned intsc_ccd_offset;
> + int sc_hz_count;
>
> - struct ksensor sc_sensor;
> struct ksensordev sc_sensordev;
> + struct ksensor sc_sensor; /* Tctl */
> + struct ksensor sc_ccd_sensor[12]; /* Tccd */
> +
> + struct timeout sc_hz_timeout;
> + struct ksensor *sc_hz_sensor;
> + struct cpu_info **sc_hz_cpu_info;
> };
>
> int ksmn_match(struct device *, void *, void *);
> void ksmn_attach(struct device *, struct device *, void *);
> +uint32_t ksmn_read_reg(struct ksmn_softc *, uint32_t);
> +void ksmn_ccd_attach(struct ksmn_softc *, int);
> +void ksmn_hz_task(void *);
> void ksmn_refresh(void *);
>
> const struct cfattach ksmn_ca = {
> @@ -113,7 +128,12 @@ ksmn_attach(struct device *parent, struc
> struct ksmn_softc *sc = (struct ksmn_softc *)self;
> struct pci_attach_args *pa = aux;
> struct curtmp_offset*p;
> - extern char cpu_model[];
> + CPU_INFO_ITERATORcii;
> + struct cpu_info *ci = curcpu();
> + struct ksensor *s;
> + extern char cpu_model[];
> + int i;
> +
>
> sc->sc_pc = pa->pa_pc;
> sc->sc_pcitag = pa->pa_tag;
> @@ -122,6 +142,7 @@ ksmn_attach(struct device *parent, struc
> sizeof(sc->sc_sensordev.xname));
>
> sc->sc_sensor.type = SENSOR_TEMP;
> + snprintf(sc->sc_sensor.desc, sizeof(sc->sc_sensor.desc), "Tctl");
> sensor_attach(>sc_sensordev, >sc_sensor);
>
> /*
> @@ -136,6 +157,80 @@ ksmn_attach(struct device *parent, struc
> sc->sc_tctl_offset = p->tctl_offset;
> }
>
> + sc->sc_ccd_offset = 0x154;
> +
> + if (ci->ci_family == 0x17 || ci->ci_family == 0x18) {
> + switch (ci->ci_model) {
> + case 0x1: /* Zen */
> + case 0x8: /* Zen+ */
> + case 0x11: /* Zen APU */
> + case 0x18: /* Zen+ APU */
> + ksmn_ccd_attach(sc, 4);
> + break;
> + case 0x31: /* Zen2 Threadripper */
> + case 0x60: /* Renoir
Re: VMM avoid duplication and reduce atack surface with octboot(4)
On Wed, Mar 23, 2022 at 04:27:40AM +, Alexis wrote: > Indeed I understood both octboot and vmm seabios/uefi initialazation process. > > But has its done with kexec and linuxboot coreboot payload octboot could be > ported to act in the same way. Explaining now again for the 3rd time, to > avoid stack duplication and decrease atack surface. Basically only openbsd > kernel network drivers etc would be needed not all equivalent seabios/uefi > ones ... seabios has no networking at all. What "attack surface" are you even talking about.. vmm/vmd has no uefi firmware available. > But the problem here is one of ego because Philipe for one to understand > that, people need to check link provided before falling into unbased > assumptions. I also understand this is a major project nontheless this was > the idea and vision I intended to share with original post Yes, clearly someone has made some major assumptions worsened yet by having not read any of the code.
Re: ps STAT sorted
On Tue, Feb 08, 2022 at 08:39:35PM +0100, Alexander Bluhm wrote:
> Hi,
>
> Sort the ps(1) STAT characters alphabetically like in the man page.
> Note that the 'else' I have removed is redundant.
>
> ok?
>
> bluhm
>
> Index: bin/ps/print.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/bin/ps/print.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 print.c
> --- bin/ps/print.c7 Feb 2022 22:57:47 - 1.80
> +++ bin/ps/print.c8 Feb 2022 19:21:07 -
> @@ -262,35 +262,35 @@ printstate(const struct kinfo_proc *kp,
> }
> cp++;
>
> + if ((kp->p_psflags & PS_CONTROLT) && kp->p__pgid == kp->p_tpgid)
> + *cp++ = '+';
> if (kp->p_nice < NZERO)
> *cp++ = '<';
> - else if (kp->p_nice > NZERO)
> - *cp++ = 'N';
> - if (kp->p_psflags & PS_TRACED)
> - *cp++ = 'X';
> + if ((flag & P_SYSTEM) == 0 &&
> + kp->p_rlim_rss_cur / 1024 < pgtok(kp->p_vm_rssize))
> + *cp++ = '>';
> + if (kp->p_eflag & EPROC_CHROOT)
> + *cp++ = 'c';
> if ((kp->p_psflags & (PS_EXITING | PS_ZOMBIE)) == PS_EXITING)
> *cp++ = 'E';
> - if (kp->p_psflags & PS_ISPWAIT)
> - *cp++ = 'V';
> if (flag & P_SYSTEM)
> *cp++ = 'K';
> - if ((flag & P_SYSTEM) == 0 &&
> - kp->p_rlim_rss_cur / 1024 < pgtok(kp->p_vm_rssize))
> - *cp++ = '>';
> - if (kp->p_eflag & EPROC_SLEADER)
> - *cp++ = 's';
> - if ((kp->p_psflags & PS_CONTROLT) && kp->p__pgid == kp->p_tpgid)
> - *cp++ = '+';
> + if (kp->p_nice > NZERO)
> + *cp++ = 'N';
> if (kp->p_psflags & PS_PLEDGE)
> *cp++ = 'p';
> + if (kp->p_eflag & EPROC_SLEADER)
> + *cp++ = 's';
> if (kp->p_eflag & EPROC_UNVEIL) {
> if (kp->p_eflag & EPROC_LKUNVEIL)
> *cp++ = 'U';
> else
> *cp++ = 'u';
> }
> - if (kp->p_eflag & EPROC_CHROOT)
> - *cp++ = 'c';
> + if (kp->p_psflags & PS_ISPWAIT)
> + *cp++ = 'V';
> + if (kp->p_psflags & PS_TRACED)
> + *cp++ = 'X';
> *cp = '\0';
>
> if (state == 'R' && kp->p_cpuid != KI_NOCPU)
I feel like shuffling the order these are printed has a higher chance of
causing confusion than additional characters. But maybe it's worth it
for new users, hmm.
-Bryan.
Re: vmd(8): fix broken bootorder for cdrom
On Thu, Nov 04, 2021 at 02:44:18PM +0100, Jan Klemkow wrote: > Hi, > > This fix [1] in seabios breaks our "boot device cdrom" feature. > > # vmctl start -Lc -d disk.img -r cd70.iso -B cdrom vm > ... > No bootable device. Retrying in 60 seconds. > > # vmctl start -Lc -d disk.img -r cd70.iso vm > doas vmctl start -c -r cd70.iso vm > ... > CD-ROM: E0 > Loading /7.0/AMD64/CDBOOT > probing: pc0 com0 mem[638K 510M a20=on] > disk: cd0 > >> OpenBSD/amd64 CDBOOT 3.53 > boot> > > The diff below, fixes the lun number of the bootorder string for cdrom. > > OK? > > bye, > Jan > > [1]: > https://github.com/coreboot/seabios/commit/f3ca59c6f3da0f30110ca216b072f8b602313734#diff-fb210bc834fdee64b4c337a57a96d0593afd9c86e956f5ef12a437810ebe61d1 > > Index: fw_cfg.c > === > RCS file: /cvs/src/usr.sbin/vmd/fw_cfg.c,v > retrieving revision 1.3 > diff -u -p -r1.3 fw_cfg.c > --- fw_cfg.c 16 Jun 2021 16:55:02 - 1.3 > +++ fw_cfg.c 4 Nov 2021 13:33:37 - > @@ -80,7 +80,7 @@ fw_cfg_init(struct vmop_create_params *v > bootorder = "/pci@i0cf8/*@2\nHALT"; > break; > case VMBOOTDEV_CDROM: > - bootorder = "/pci@i0cf8/*@4/*@0/*@0,100\nHALT"; > + bootorder = "/pci@i0cf8/*@4/*@0/*@0,4100\nHALT"; > break; > case VMBOOTDEV_NET: > /* XXX not yet */ > I noticed this was broken too after a recent discussion on twitter. ok brynet@
Re: head(1): fully support the legacy -count syntax
On Sun, Oct 10, 2021 at 02:26:32PM -0500, Scott Cheloha wrote: > On Sun, Oct 10, 2021 at 12:31:22PM -0600, Theo de Raadt wrote: > > Bryan Steele wrote: > > > > > On Sun, Oct 10, 2021 at 12:18:55PM -0500, Scott Cheloha wrote: > > > > On Sun, Oct 10, 2021 at 10:51:29AM -0600, Theo de Raadt wrote: > > > > > did anyone ever use it this way, or are you getting ahead of yourself. > > > > > > > > I don't understand the question. > > > > > > I've only ever seen it used with -count as the first argument, can't > > > say it's every occoured to me to type "head file -10". > > That is not what I proposed. Reread my first message: > > https://marc.info/?l=openbsd-tech=163388435528203=2 Yes, sorry. It's been a day.. > > POSIX says options before arguments. That is what we support. We don't > > support options after arguments. > > Yep. >
Re: head(1): fully support the legacy -count syntax
On Sun, Oct 10, 2021 at 12:18:55PM -0500, Scott Cheloha wrote: > On Sun, Oct 10, 2021 at 10:51:29AM -0600, Theo de Raadt wrote: > > did anyone ever use it this way, or are you getting ahead of yourself. > > I don't understand the question. I've only ever seen it used with -count as the first argument, can't say it's every occoured to me to type "head file -10". > The -count syntax was fully supported in the first revision of head(1): > > https://svnweb.freebsd.org/csrg/usr.bin/head/head.c?view=markup=1026 > > The -count syntax was fully supported through 4.4BSD: > > https://svnweb.freebsd.org/csrg/usr.bin/head/head.c?revision=69237=markup > > The -count syntax was also standard in SUSv2: > > https://pubs.opengroup.org/onlinepubs/7908799/xcu/head.html > > ... and then dropped in SUSv3 (POSIX-2001): > > https://pubs.opengroup.org/onlinepubs/009695399/utilities/head.html > > FreeBSD maintains full support for the -count syntax: > > https://cgit.freebsd.org/src/tree/usr.bin/head/head.c#n191 > > ... so clearly people have used it this way. If we're going to > support the -count syntax at all, why not fully support it? We can do > so with very little code. > >
Re: Stylistic Cleanup Removing Magic Numbers for STDIN_FILENO
On Sun, Apr 25, 2021 at 08:58:35PM +, Smccalib wrote: > Greetings, > > While reading the source code for ed, I noticed a small stylistic > inconsistency in main.c whereby "isatty(0)" is called followed by a > series of calls to functions that take a fd being called with > STDIN_FILENO. Although this has little impact on readability, and > should make no difference in effect, for the purpose of stylistic > consistency I scanned the tree for similar calls, and composed a series > of diffs. > > As this would be my first patch, any feedback is appreciated. > > Thanks! diffs should be in unified format (diff -u), and more specifically cvs diff -u.
Re: vmm(4): improve {rd,wr}msr exit handling for both amd & intel
On Mon, Apr 05, 2021 at 09:54:14AM -0400, Dave Voutila wrote:
>
> Dave Voutila writes:
>
> > The following diff cleans up and improves MSR-related event handling in
> > vmm(4) for when the guest attempts a rdmsr/wrmsr instruction. As
> > mentioned in a previous email to tech@ about fixing support for 9front
> > guests [1], we found some discprepencies between vmm(4)'s handling on
> > Intel hosts and AMD hosts.
> >
> > While the diff has been tested already by abieber@ and brynet@ with
> > additional review by mlarkin@, I'm looking for additional testers
> > willing to look for regressions.
>
> Last call for additional testers. Plan is to commit the diff later today
> as I have an OK from mlarkin@.
This is ok brynet@ too!
> >
> > This diff specifically improves and standardizes msr-based exit handling
> > between Intel and AMD hosts to the following:
> >
> > 1. All RDMSR instructions that cause a vm-exit must be explicitly
> >handled (e.g. via emulation) or they result in injecting a #GP
> >exception into the guest.
> >
> > 2. All WRMSR instructions that cause a vm-exit and are not explicitly
> >handled are ignored (i.e. %rax and %rdx values are not inspected or
> >used).
> >
> > Consequently with the change for (1) above, the diff adds explicit
> > handling for the following MSRs:
> >
> > 1. MSR_CR_PAT: for now reads/writes are shadowed for the guest vcpu and
> >host state is not touched. The shadow state is initialized on vcpu
> >reset to the same value as the host.
> >
> > 2. MSR_BIOS_SIGN, MSR_INT_PEN_MSG (on AMD), and MSR_PLATFORM_ID are all
> >ignored. This means reads result in vmm(4) setting the guest vcpu's
> >%rax and %rdx to 0. (These msr's are ignored in the same manner by
> >other hypervisors like kvm and nvmm.)
> >
> > The average user should not see a change in behavior of vmm(4) or
> > vmd(8). The biggest change is for *Intel* users as this diff changes the
> > current vmx logic which was not injecting #GP for unsupported
> > msr's. (Your guests were potentially getting garbage results from rdmsr
> > instructions.)
> >
>
> If you do test the diff and have issues, I forgot to mention to please
> build the kernel with VMM_DEBUG. The output to the kernel buffer will
> help diagnose any problematic msr access.
>
> > The folks attempting to host the latest release of 9front as a guest on
> > AMD hosts should see their guest boot successfully with this diff :-)
> >
> > -dv
> >
> > [1] https://marc.info/?l=openbsd-tech=161693517121814=2
> >
> >
> > Index: sys/arch/amd64/include/vmmvar.h
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/include/vmmvar.h,v
> > retrieving revision 1.70
> > diff -u -p -r1.70 vmmvar.h
> > --- sys/arch/amd64/include/vmmvar.h 8 Apr 2020 07:39:48 - 1.70
> > +++ sys/arch/amd64/include/vmmvar.h 31 Mar 2021 00:15:43 -
> > @@ -936,6 +936,9 @@ struct vcpu {
> > paddr_t vc_pvclock_system_gpa;
> > uint32_t vc_pvclock_system_tsc_mul;
> >
> > + /* Shadowed MSRs */
> > + uint64_t vc_shadow_pat;
> > +
> > /* VMX only */
> > uint64_t vc_vmx_basic;
> > uint64_t vc_vmx_entry_ctls;
> > Index: sys/arch/amd64/amd64/vmm.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> > retrieving revision 1.278
> > diff -u -p -r1.278 vmm.c
> > --- sys/arch/amd64/amd64/vmm.c 11 Mar 2021 11:16:55 - 1.278
> > +++ sys/arch/amd64/amd64/vmm.c 31 Mar 2021 00:15:43 -
> > @@ -207,6 +207,7 @@ void svm_set_dirty(struct vcpu *, uint32
> > int vmm_gpa_is_valid(struct vcpu *vcpu, paddr_t gpa, size_t obj_size);
> > void vmm_init_pvclock(struct vcpu *, paddr_t);
> > int vmm_update_pvclock(struct vcpu *);
> > +int vmm_pat_is_valid(uint64_t);
> >
> > #ifdef VMM_DEBUG
> > void dump_vcpu(struct vcpu *);
> > @@ -3193,6 +3194,9 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
> > /* xcr0 power on default sets bit 0 (x87 state) */
> > vcpu->vc_gueststate.vg_xcr0 = XCR0_X87 & xsave_mask;
> >
> > + /* XXX PAT shadow */
> > + vcpu->vc_shadow_pat = rdmsr(MSR_CR_PAT);
> > +
> > exit:
> > /* Flush the VMCS */
> > if (vmclear(>vc_control_pa)) {
> > @@ -3584,6 +3588,10 @@ vcpu_init(struct vcpu *vcpu)
> > vcpu->vc_state = VCPU_STATE_STOPPED;
> > vcpu->vc_vpid = 0;
> > vcpu->vc_pvclock_system_gpa = 0;
> > +
> > + /* Shadow PAT MSR, starting with host's value. */
> > + vcpu->vc_shadow_pat = rdmsr(MSR_CR_PAT);
> > +
> > if (vmm_softc->mode == VMM_MODE_VMX ||
> > vmm_softc->mode == VMM_MODE_EPT)
> > ret = vcpu_init_vmx(vcpu);
> > @@ -6257,26 +6265,24 @@ vmx_handle_rdmsr(struct vcpu *vcpu)
> > rdx = >vc_gueststate.vg_rdx;
> >
> > switch (*rcx) {
> > - case MSR_SMBASE:
> > - /*
> > -* 34.15.6.3 - Saving Guest State (SMM)
> > -*
> > -* Unsupported, so inject #GP and return without
> > -
Re: vmm(4): fix boot issue for 9front guests
On Sun, Mar 28, 2021 at 08:38:13AM -0400, Dave Voutila wrote:
> abieber@ found the latest 9front release ends up in a boot loop if
> hosted on an AMD system. I tracked it down to 9front (oddly) trying to
> read the PAT msr prior to writing it. [1] The problem is vmm(4)'s msr
> handling for svm injects #GP exceptions into the guest for most msr
> reads (since we don't emulate more than a few).
>
> For those (two? few? dozen?) 9front users of AMD hardware and -current,
> can you try the below diff?
>
> vmm(4)'s vmx msr handlers ignores this instruction and only logs the
> rdmsr information if the kernel is built with VMM_DEBUG. vmm(4) will
> advance the instruction pointer regardless and it's up to the guest to
> deal with any resulting issues.
>
> The diff syncs the logic between the svm and vmx msr vm-exit handlers by
> injecting #GP *ONLY* on attempts to read the SMBASE msr.
>
> For context, this is the vmx rdmsr handler's (vmx_handle_rdmsr) logic:
>
> switch (*rcx) {
> case MSR_SMBASE:
> /*
>* 34.15.6.3 - Saving Guest State (SMM)
>*
>* Unsupported, so inject #GP and return without
>* advancing %rip.
>*/
> ret = vmm_inject_gp(vcpu);
> return (ret);
> }
>
> It is *not* a design for emulating PAT access and manipulation by a
> guest.
>
> (As an aside, OpenBSD doesn't bother reading the msr [2] before writing
> to it, neither does Linux. Why is 9front special? ¯\_(ツ)_/¯)
>
> -Dave
>
> [1] https://code.9front.org/hg/plan9front/rev/10cd3e23a8c1
> [2]
> https://github.com/openbsd/src/blob/36fd90dcf1acf2ddb4ef5dbabe5313b3a8d46ee2/sys/arch/amd64/amd64/cpu.c#L1145-L1168
>
>
> Index: sys/arch/amd64/amd64/vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.278
> diff -u -p -r1.278 vmm.c
> --- sys/arch/amd64/amd64/vmm.c11 Mar 2021 11:16:55 - 1.278
> +++ sys/arch/amd64/amd64/vmm.c28 Mar 2021 00:45:08 -
> @@ -6545,10 +6545,16 @@ svm_handle_msr(struct vcpu *vcpu)
> *rax = 0;
> *rdx = 0;
> break;
> - default:
> - DPRINTF("%s: guest read msr 0x%llx, injecting "
> - "#GP\n", __func__, *rcx);
> + case MSR_SMBASE:
> + /* Unsupported, inject #GP w/o advancing %rip */
AMD makes no mention of this MSR, I think it's Intel, AMD's documentation has
SMBASE MSR as C001_0111h. "Table A-1. MSRs of the AMD64 Architecture"
pg. 654. https://www.amd.com/system/files/TechDocs/24593.pdf
> ret = vmm_inject_gp(vcpu);
> return (ret);
> +#ifdef VMM_DEBUG
> + default:
> + /* Log the access to identify unknown MSRs */
> + DPRINTF("%s: rdmsr exit, msr=0x%llx, data "
> + "returned to guest=0x%llx:0x%llx\n",
> + __func__, *rcx, *rdx, *rax);
> +#endif /* VMM_DEBUG */
> }
> }
Re: vmm(4): fix boot issue for 9front guests
On Sun, Mar 28, 2021 at 08:38:13AM -0400, Dave Voutila wrote:
> abieber@ found the latest 9front release ends up in a boot loop if
> hosted on an AMD system. I tracked it down to 9front (oddly) trying to
> read the PAT msr prior to writing it. [1] The problem is vmm(4)'s msr
> handling for svm injects #GP exceptions into the guest for most msr
> reads (since we don't emulate more than a few).
>
> For those (two? few? dozen?) 9front users of AMD hardware and -current,
> can you try the below diff?
>
> vmm(4)'s vmx msr handlers ignores this instruction and only logs the
> rdmsr information if the kernel is built with VMM_DEBUG. vmm(4) will
> advance the instruction pointer regardless and it's up to the guest to
> deal with any resulting issues.
>
> The diff syncs the logic between the svm and vmx msr vm-exit handlers by
> injecting #GP *ONLY* on attempts to read the SMBASE msr.
>
> For context, this is the vmx rdmsr handler's (vmx_handle_rdmsr) logic:
>
> switch (*rcx) {
> case MSR_SMBASE:
> /*
>* 34.15.6.3 - Saving Guest State (SMM)
>*
>* Unsupported, so inject #GP and return without
>* advancing %rip.
>*/
> ret = vmm_inject_gp(vcpu);
> return (ret);
> }
>
> It is *not* a design for emulating PAT access and manipulation by a
> guest.
>
> (As an aside, OpenBSD doesn't bother reading the msr [2] before writing
> to it, neither does Linux. Why is 9front special? ¯\_(ツ)_/¯)
>
> -Dave
>
> [1] https://code.9front.org/hg/plan9front/rev/10cd3e23a8c1
> [2]
> https://github.com/openbsd/src/blob/36fd90dcf1acf2ddb4ef5dbabe5313b3a8d46ee2/sys/arch/amd64/amd64/cpu.c#L1145-L1168
>
>
> Index: sys/arch/amd64/amd64/vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.278
> diff -u -p -r1.278 vmm.c
> --- sys/arch/amd64/amd64/vmm.c11 Mar 2021 11:16:55 - 1.278
> +++ sys/arch/amd64/amd64/vmm.c28 Mar 2021 00:45:08 -
> @@ -6545,10 +6545,16 @@ svm_handle_msr(struct vcpu *vcpu)
> *rax = 0;
> *rdx = 0;
> break;
> - default:
> - DPRINTF("%s: guest read msr 0x%llx, injecting "
> - "#GP\n", __func__, *rcx);
> + case MSR_SMBASE:
> + /* Unsupported, inject #GP w/o advancing %rip */
> ret = vmm_inject_gp(vcpu);
> return (ret);
> +#ifdef VMM_DEBUG
> + default:
> + /* Log the access to identify unknown MSRs */
> + DPRINTF("%s: rdmsr exit, msr=0x%llx, data "
> + "returned to guest=0x%llx:0x%llx\n",
> + __func__, *rcx, *rdx, *rax);
> +#endif /* VMM_DEBUG */
> }
> }
I'm not sure this is correct, doesn't this mean that registers will
contain whatevever garbage that was in them beforehand, without
injecting #GP host does the guest kernel to know the MSR read failed?
I was initially concerned as this touches the codepath pd@ fixed last
Feb where MSR reads were being passed through to the host, but still
I think that injecting the #GP for unsupported MSR reads is right.
-Bryan.
Re: wsconsctl.conf: mention mouse.tp.tapping in example
On Mon, Mar 22, 2021 at 08:18:45PM +0100, Klemens Nanni wrote: > I was too stupid to look at `wsconsctl' output (which needs root) and > only looked here. > > Mailing the diff for my lack of better wording, plus the knob atually > takes three values which I have yet to decode by reading wsconsctl(8) > code. Appears to be documented here: https://man.openbsd.org/wsmouse.4#mouse.tp.tapping > Anyone with a better text for the example so grepping "mouse" or "tap" > shows me what I'm looking for? > > > Index: examples/wsconsctl.conf > === > RCS file: /cvs/src/etc/examples/wsconsctl.conf,v > retrieving revision 1.1 > diff -u -p -r1.1 wsconsctl.conf > --- examples/wsconsctl.conf 16 Jul 2014 13:21:33 - 1.1 > +++ examples/wsconsctl.conf 22 Mar 2021 19:15:52 - > @@ -9,3 +9,4 @@ > #display.vblank=on # enable vertical sync blank for screen burner > #display.screen_off=6# set screen burner timeout to 60 seconds > #display.msact=off # disable screen unburn w/ mouse > +#mouse.tp.tapping=0 # 1=click on touch not physical push > >
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>
>
> > On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
> >
> >>> Created a fresh install qcow2 image and derived 35 new VMs from it.
> >>> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting
> >>> 240 seconds after each cycle.
> >>> Similar to the staggered start based on the amount of CPUs.
> >
> >> For me this is not enough info to even try to reproduce, I know little
> >> of vmm or vmd and have no idea what "derive" means in this context.
> >
> > This is a big bit of information that was missing from the original
>
> Well.. could have been better described indeed. :))
> " I created 41 additional VMs based on a single qcow2 base image.”
>
> > report ;) qcow has a concept of a read-only base image (or 'backing
> > file') which can be shared between VMs, with writes diverted to a
> > separate image ('derived image').
> >
> > So e.g. you can create a base image, do a simple OS install for a
> > particular OS version to that base image, then you stop using that
> > for a VM and just use it as a base to create derived images from.
> > You then run VMs using the derived image and make whatever config
> > changes. If you have a bunch of VMs using the same OS release then
> > you save some disk space for the common files.
> >
> > Mischa did you leave a VM running which is working on the base
> > image directly? That would certainly cause problems.
>
> I did indeed. Let me try that again without keeping the base image running.
>
> Mischa
I seemed to recall that the base image is not supposed to be modified,
so this is a pretty big omission.
Per original commit message:
"A limitation of this format is that modifying the base image will
corrupt the derived image."
https://marc.info/?l=openbsd-cvs=153901633011716=2
-Bryan.
Re: occasional SSIGSEGV on C++ exception handling
On Tue, Feb 23, 2021 at 06:23:22PM +1100, Jonathan Gray wrote: > On Tue, Feb 23, 2021 at 08:10:54AM +0100, Otto Moerbeek wrote: > > On Mon, Feb 22, 2021 at 08:58:07PM -, Miod Vallat wrote: > > > > > > > > > No problem, real-life often takes precedence. > > > > > > No way! operator(7) would need an update! > > > > > > > What do we do when we see a bug? We fix it! What if it is not fixable? > > We document it! > > > > -Otto > > real life is not a C operator Well I guess that depends on how one lives their life, you may need to C an operator now and then. > > > > Index: operator.7 > > === > > RCS file: /cvs/src/share/man/man7/operator.7,v > > retrieving revision 1.11 > > diff -u -p -r1.11 operator.7 > > --- operator.7 21 Jun 2019 02:28:34 - 1.11 > > +++ operator.7 23 Feb 2021 07:09:10 - > > @@ -57,3 +57,5 @@ > > .It "\&," Ta "left to right" > > .El > > .Ed > > +.Sh BUGS > > +Often real life takes precedence. > > > > > >
Re: uhidpp(4): logitech hid++ device driver
On Tue, Feb 02, 2021 at 08:23:29AM +0100, Anton Lindqvist wrote: > On Sat, Jan 30, 2021 at 01:18:07PM +0200, Ville Valkonen wrote: > > On Sat, 2021-01-30 at 08:36 +0100, Anton Lindqvist wrote: > > > On Fri, Jan 29, 2021 at 10:15:05PM +0200, Ville Valkonen wrote: > > > > Hi, > > > > > > > > I have a bit oldish Logitech M705 mouse, bought around 2010-2011. > > > > Regarding the dmesg (on below) I can see it gets attached correctly > > > > to > > > > uhiddp0 but doesn't report battery levels. Here's the line from > > > > dmesg: > > > > uhidpp0 at uhidev2 device 1 mouse "M705" serial xx-xx-x-xx, device > > > > 2 keyboard "K750" serial xx-xx-xx-xx. > > > > And corresponding sysctl values: > > > > hw.sensors.uhidpp0.raw0=unknown (battery levels) > > > > hw.sensors.uhidpp0.raw1=unknown (battery levels) > > > > hw.sensors.uhidpp0.percent0=unknown (battery level) > > > > hw.sensors.uhidpp0.percent1=unknown (battery level) > > > > > > > > Not sure if censoring of serial has any value, though. > > > > > > Glad to see it attaches fine to a receiver with more than one device > > > paired. I only have one device myself and have therefore never been > > > enable to verify this. > > > > > > Could you enable UHIDPP_DEBUG and send me the output? > > > > > > > On Ubuntu battery levels are detected correctly so I could probably > > > > take a USB dump with Wireshark and compare the differences. > > > > > > Taking a USB dump on your Linux machine would be very helpful. > > > > Hi, > > > > Yes. Also remembered that you were mentioning about the debug flag but > > completely forgot that while testing. Then just before going to sleep > > recalled the debug flag. Here are the results with debug enabled: > > > > uhidev0 at uhub0 port 1 configuration 1 interface 0 "Logitech USB Receiver" > > rev 2.00/12.10 addr 3 > > uhidev0: iclass 3/1 > > ukbd0 at uhidev0: 8 variable keys, 6 key codes > > wskbd1 at ukbd0 mux 1 > > wskbd1: connecting to wsdisplay0 > > uhidev1 at uhub0 port 1 configuration 1 interface 1 "Logitech USB Receiver" > > rev 2.00/12.10 addr 3 > > uhidev1: iclass 3/1, 8 report ids > > ums0 at uhidev1 reportid 2: 16 buttons, Z and W dir > > wsmouse2 at ums0 mux 0 > > uhid0 at uhidev1 reportid 3: input=4, output=0, feature=0 > > uhid1 at uhidev1 reportid 4: input=1, output=0, feature=0 > > uhid2 at uhidev1 reportid 8: input=1, output=0, feature=0 > > uhidev2 at uhub0 port 1 configuration 1 interface 2 "Logitech USB Receiver" > > rev 2.00/12.10 addr 3 > > uhidev2: iclass 3/0, 33 report ids > > uhidpp0 at uhidev2hidpp_send_report: 10 ff 83 b5 [30 00 00] > > uhidpp_intr: 11 ff 83 b5 [30 c4 b4 96 9e 04 00 00 00 01 00 00 00 00 00 00] > > hidpp_send_report: 10 ff 83 b5 [20 00 00] > > uhidpp_intr: 11 ff 83 b5 [20 09 08 10 1b 04 00 02 06 00 00 00 00 00 00 00] > > hidpp_send_report: 10 ff 83 b5 [40 00 00] > > uhidpp_intr: 11 ff 83 b5 [40 04 4d 37 30 35 00 00 00 00 00 00 00 00 00 00] > > device 1 mouse "M705" serial xx-xx-xx-9ehidpp_send_report: 10 ff 83 b5 [31 > > 00 00] > > uhidpp_intr: 11 ff 83 b5 [31 8d 37 6a 6f 1a 40 00 00 03 00 00 00 00 00 00] > > hidpp_send_report: 10 ff 83 b5 [21 00 00] > > uhidpp_intr: 11 ff 83 b5 [21 08 14 40 02 04 00 01 07 00 00 00 00 00 00 00] > > hidpp_send_report: 10 ff 83 b5 [41 00 00] > > uhidpp_intr: 11 ff 83 b5 [41 04 4b 37 35 30 00 00 00 00 00 00 00 00 00 00] > > , device 2 keyboard "K750" serial xx-xx-xx-6fhidpp_send_report: 10 ff 83 b5 > > [32 00 00] > > uhidpp_intr: 10 ff 8f 83 [b5 03 00] > > hidpp_send_report: 10 ff 83 b5 [33 00 00] > > uhidpp_intr: 10 ff 8f 83 [b5 03 00] > > hidpp_send_report: 10 ff 83 b5 [34 00 00] > > uhidpp_intr: 10 ff 8f 83 [b5 03 00] > > hidpp_send_report: 10 ff 83 b5 [35 00 00] > > uhidpp_intr: 10 ff 8f 83 [b5 03 00] > > hidpp_send_report: 10 ff 80 00 [10 09 00] > > uhidpp_intr: 10 ff 80 00 [00 00 00] > > > > > > That's when the mouse was off. When I switched on the mouse kernel > > panicked. I couldn't break into DDB or alternatively failed to type > > correct commands blindly. Eventually had to shutdown the system by > > pressing the power button. Picture of the panic is here: > > https://imgur.com/a/QRAD5v1 > > Thanks for the report. Updated diff which should fix the panic: > > * Fix a bug in uhidpp_is_notification() causing notifications to be > detected as responses > > * Delay installation of sensors > > * Enable uhidpp on all architectures > > >From the panic, I can see that your device only supports HID++ 1.0. > Querying the battery status works a bit differently compared to HID++ > 2.0. I don't have a 1.0 device but can probably give this a try if > you're willing to try out future diffs. > > However, it would ease development by getting this in and continue > development in tree. Anyone willing to ok? > > > Btw. Time has passed since my previous kernel compile. I saw the > > procedure has changed a bit since then. I initially tried to compile > > debug flags by prepending `option UHIDPP_DEBUG` to > > sys/arch/amd64/compile/GENERIC.MP
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 02:26:34PM -0500, Bryan Steele wrote: > On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote: > > On Fri, 15 Jan 2021 22:41:13 +0100 > > Marcus Glocker wrote: > > > > > On Fri, 15 Jan 2021 11:37:47 -0500 > > > Bryan Steele wrote: > > > > > > > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote: > > > > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > > > > > > > > > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote: > > > > > > > > > > > > > > I have heard from others who tried the diff that the PS4 > > > > > > > > controller is causing problems with the way it attaches. I > > > > > > > > ordered one to trial-and- error this myself at home. Could > > > > > > > > you share output of lsusb -vv? Thanks for giving it a try! > > > > > > > > > > > > > > > > > > > > > > Sure, here we go. > > > > > > > If I can find anything myself in the meantime I let you know. > > > > > > > > > > > > > > > > > > > So the problem doesn't seem to be in your new ujoy(4) code, but > > > > > > how the dev/hid/hid.c:hid_is_collection() function tries to cope > > > > > > with the PS4 controller. > > > > > > > > > > So with the hid_is_collection() problem not easy to mitigate [1], > > > > > should we table the ujoy(4) proposal for now pending a fix for the > > > > > problems with the PS4 controller? Or is this interesting enough > > > > > for some to work on moving forward despite this issue and finding > > > > > a solution for this specific (and in some ways unusual) device > > > > > later? > > > > > > > > Considering the hid_is_collection() fix from NetBSD proposed by > > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers > > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now? > > > > It's fairly small helper function. Like hid_is_joy_collection()? > > > > > > I think that could be an XXX workaround until somebody finds a proper, > > > generic fix for hid_is_collection() ... > > > > That's what you meant, yes? > > Yep, this is what I meant. Hopefully it won't have to stay for long, but > t would be nice to get ujoy(4) working as many controllers as possible > so that it can go in. > > Can you send a new full diff thfr@? I still need to test with my USB > SNES controller, but tentatively this is still ok brynet@ > > -Bryan. Yep, works fine with my generic USB SNES controller, tested with usbhidctl -f /dev/ujoy/0 -l, shows all button presses. uhidev4 at uhub0 port 12 configuration 1 interface 0 "vendor 0x0810 usb gamepad" rev 1.00/1.06 addr 2 uhidev4: iclass 3/0, 1 report id ujoy0 at uhidev4 reportid 1: input=7, output=0, feature=0 I think this is ready to go in. :-) -Bryan.
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
> On Fri, 15 Jan 2021 22:41:13 +0100
> Marcus Glocker wrote:
>
> > On Fri, 15 Jan 2021 11:37:47 -0500
> > Bryan Steele wrote:
> >
> > > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> > > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > > >
> > > > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > > > >
> > > > > > > I have heard from others who tried the diff that the PS4
> > > > > > > controller is causing problems with the way it attaches. I
> > > > > > > ordered one to trial-and- error this myself at home. Could
> > > > > > > you share output of lsusb -vv? Thanks for giving it a try!
> > > > > > >
> > > > > >
> > > > > > Sure, here we go.
> > > > > > If I can find anything myself in the meantime I let you know.
> > > > > >
> > > > >
> > > > > So the problem doesn't seem to be in your new ujoy(4) code, but
> > > > > how the dev/hid/hid.c:hid_is_collection() function tries to cope
> > > > > with the PS4 controller.
> > > >
> > > > So with the hid_is_collection() problem not easy to mitigate [1],
> > > > should we table the ujoy(4) proposal for now pending a fix for the
> > > > problems with the PS4 controller? Or is this interesting enough
> > > > for some to work on moving forward despite this issue and finding
> > > > a solution for this specific (and in some ways unusual) device
> > > > later?
> > >
> > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > It's fairly small helper function. Like hid_is_joy_collection()?
> >
> > I think that could be an XXX workaround until somebody finds a proper,
> > generic fix for hid_is_collection() ...
>
> That's what you meant, yes?
Yep, this is what I meant. Hopefully it won't have to stay for long, but
t would be nice to get ujoy(4) working as many controllers as possible
so that it can go in.
Can you send a new full diff thfr@? I still need to test with my USB
SNES controller, but tentatively this is still ok brynet@
-Bryan.
> Can this been updated in the overall diff and re-verified if it still
> works fine with the other controllers? It (obviously) works with my
> PS4 controller.
>
>
> Index: sys/dev/usb/ujoy.c
> ===
> RCS file: sys/dev/usb/ujoy.c
> diff -N sys/dev/usb/ujoy.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sys/dev/usb/ujoy.c22 Jan 2021 17:54:46 -
> @@ -0,0 +1,149 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2020 Thomas Frohwein
> + * Copyright (c) 2020 Bryan Steele
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +int ujoy_match(struct device *, void *, void *);
> +
> +struct cfdriver ujoy_cd = {
> + NULL, "ujoy", DV_DULL
> +};
> +
> +const struct cfattach ujoy_ca = {
> + sizeof(struct uhid_softc),
> + ujoy_match,
> + uhid_attach,
> + uhid_detach,
> +};
> +
> +/*
> + * XXX workaround:
> + *
> + * This is a copy of sys/dev/hid/hid.c:hid_is_collection(), synced up to the
> + * NetBSD version. Our
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote: > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote: > > > > > > I have heard from others who tried the diff that the PS4 controller is > > > > causing problems with the way it attaches. I ordered one to trial-and- > > > > error this myself at home. Could you share output of lsusb -vv? Thanks > > > > for giving it a try! > > > > > > Sure, here we go. > > > If I can find anything myself in the meantime I let you know. > > > > So the problem doesn't seem to be in your new ujoy(4) code, but how the > > dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4 > > controller. > > So with the hid_is_collection() problem not easy to mitigate [1], > should we table the ujoy(4) proposal for now pending a fix for the > problems with the PS4 controller? Or is this interesting enough for > some to work on moving forward despite this issue and finding a > solution for this specific (and in some ways unusual) device later? Considering the hid_is_collection() fix from NetBSD proposed by Marcus fixes the issue with ujoy(4) but breaks some other drivers (imt(4) and ims(4)), could we not inline it into ujoy(4) for now? It's fairly small helper function. Like hid_is_joy_collection()? -Bryan. > 3-4 have tested and reported to me so far. It seems so far that the > only new breakage is with the PS4 controller, and there is probably > another solution that can be found later that doesn't break other > drivers like [1]? > > [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox > > > > > I'm not much in to HID, but when I sync up the hid_is_collection() > > function with NetBSD, the PS4 controller attaches to ujoy(4) as > > expected, and also can be accessed by usbhidctl(1), while my other > > HID devices continue to work as before: > > > > uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive > > Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls > > audio1 at uaudio0 > > uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive > > Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > uhidev6: iclass 3/0, 180 report ids > > ujoy0 at uhidev6 reportid 1: input=63, output=0, feature=0 <-- > > uhid6 at uhidev6 reportid 2: input=0, output=0, feature=36 > > uhid7 at uhidev6 reportid 4: input=0, output=0, feature=36 > > uhid8 at uhidev6 reportid 5: input=0, output=31, feature=0 > > uhid9 at uhidev6 reportid 8: input=0, output=0, feature=3 > > uhid10 at uhidev6 reportid 16: input=0, output=0, feature=4 > > uhid11 at uhidev6 reportid 17: input=0, output=0, feature=2 > > uhid12 at uhidev6 reportid 18: input=0, output=0, feature=15 > > uhid13 at uhidev6 reportid 19: input=0, output=0, feature=22 > > uhid14 at uhidev6 reportid 20: input=0, output=0, feature=16 > > uhid15 at uhidev6 reportid 21: input=0, output=0, feature=44 > > uhid16 at uhidev6 reportid 128: input=0, output=0, feature=6 > > uhid17 at uhidev6 reportid 129: input=0, output=0, feature=6 > > uhid18 at uhidev6 reportid 130: input=0, output=0, feature=5 > > uhid19 at uhidev6 reportid 131: input=0, output=0, feature=1 > > uhid20 at uhidev6 reportid 132: input=0, output=0, feature=4 > > uhid21 at uhidev6 reportid 133: input=0, output=0, feature=6 > > uhid22 at uhidev6 reportid 134: input=0, output=0, feature=6 > > uhid23 at uhidev6 reportid 135: input=0, output=0, feature=35 > > uhid24 at uhidev6 reportid 136: input=0, output=0, feature=34 > > uhid25 at uhidev6 reportid 137: input=0, output=0, feature=2 > > uhid26 at uhidev6 reportid 144: input=0, output=0, feature=5 > > uhid27 at uhidev6 reportid 145: input=0, output=0, feature=3 > > uhid28 at uhidev6 reportid 146: input=0, output=0, feature=3 > > uhid29 at uhidev6 reportid 147: input=0, output=0, feature=12 > > uhid30 at uhidev6 reportid 160: input=0, output=0, feature=6 > > uhid31 at uhidev6 reportid 161: input=0, output=0, feature=1 > > uhid32 at uhidev6 reportid 162: input=0, output=0, feature=1 > > uhid33 at uhidev6 reportid 163: input=0, output=0, feature=48 > > uhid34 at uhidev6 reportid 164: input=0, output=0, feature=13 > > uhid35 at uhidev6 reportid 165: input=0, output=0, feature=21 > > uhid36 at uhidev6 reportid 166: input=0, output=0, feature=21 > > uhid37 at uhidev6 reportid 167: input=0, output=0, feature=1 > > uhid38 at uhidev6 reportid 168: input=0, output=0, feature=1 > > uhid39 at uhidev6 reportid 169: input=0, output=0, feature=8 > > uhid40 at uhidev6 reportid 170: input=0, output=0, feature=1 > > uhid41 at uhidev6 reportid 171: input=0, output=0, feature=57 > > uhid42 at uhidev6 reportid 172: input=0, output=0, feature=57 > > uhid43 at uhidev6 reportid 173: input=0, output=0, feature=11 > > uhid44 at uhidev6 reportid 174: input=0, output=0, feature=1 > > uhid45 at uhidev6 reportid 175: input=0, output=0, feature=2 > > uhid46
Re: New ujoy(4) device for USB gamecontrollers
=== > RCS file: /cvs/src/etc/etc.landisk/MAKEDEV.md,v > retrieving revision 1.48 > diff -u -p -r1.48 MAKEDEV.md > --- etc/etc.landisk/MAKEDEV.md6 Jul 2020 06:11:27 - 1.48 > +++ etc/etc.landisk/MAKEDEV.md28 Dec 2020 03:25:05 - > @@ -65,6 +65,7 @@ _DEV(uall) > _DEV(ugen, 70) > _DEV(uhid, 65) > _DEV(fido, 106) > +_DEV(ujoy, 108) > _DEV(ulpt, 66) > _DEV(usb, 64) > _TITLE(spec) > Index: etc/etc.loongson/MAKEDEV.md > === > RCS file: /cvs/src/etc/etc.loongson/MAKEDEV.md,v > retrieving revision 1.32 > diff -u -p -r1.32 MAKEDEV.md > --- etc/etc.loongson/MAKEDEV.md 6 Jul 2020 06:11:27 - 1.32 > +++ etc/etc.loongson/MAKEDEV.md 28 Dec 2020 03:25:05 - > @@ -60,6 +60,7 @@ _DEV(uall) > _DEV(ugen, 63) > _DEV(uhid, 62) > _DEV(fido, 88) > +_DEV(ujoy, 90) > _DEV(ulpt, 64) > _DEV(usb, 61) > _TITLE(spec) > Index: etc/etc.macppc/MAKEDEV.md > === > RCS file: /cvs/src/etc/etc.macppc/MAKEDEV.md,v > retrieving revision 1.75 > diff -u -p -r1.75 MAKEDEV.md > --- etc/etc.macppc/MAKEDEV.md 6 Jul 2020 06:11:27 - 1.75 > +++ etc/etc.macppc/MAKEDEV.md 28 Dec 2020 03:25:05 - > @@ -70,6 +70,7 @@ _DEV(ttyU, 66) > _DEV(ugen, 63) > _DEV(uhid, 62) > _DEV(fido, 90) > +_DEV(ujoy, 92) > _DEV(ulpt, 64) > _DEV(usb, 61) > _TITLE(spec) > Index: etc/etc.octeon/MAKEDEV.md > === > RCS file: /cvs/src/etc/etc.octeon/MAKEDEV.md,v > retrieving revision 1.19 > diff -u -p -r1.19 MAKEDEV.md > --- etc/etc.octeon/MAKEDEV.md 6 Jul 2020 06:11:27 - 1.19 > +++ etc/etc.octeon/MAKEDEV.md 28 Dec 2020 03:25:05 - > @@ -67,6 +67,7 @@ _DEV(uall) > _DEV(usb, 61) > _DEV(uhid, 62) > _DEV(fido, 76) > +_DEV(ujoy, 78) > _TITLE(spec) > _DEV(au, 44) > _DEV(bio, 49) > Index: etc/etc.powerpc64/MAKEDEV.md > === > RCS file: /cvs/src/etc/etc.powerpc64/MAKEDEV.md,v > retrieving revision 1.6 > diff -u -p -r1.6 MAKEDEV.md > --- etc/etc.powerpc64/MAKEDEV.md 24 Oct 2020 21:10:41 - 1.6 > +++ etc/etc.powerpc64/MAKEDEV.md 28 Dec 2020 03:25:05 - > @@ -52,6 +52,7 @@ _DEV(uall) > _DEV(ugen, 49) > _DEV(uhid, 50) > _DEV(fido, 51) > +_DEV(ujoy, 94) > _DEV(ulpt, 65) > _DEV(usb, 48) > _TITLE(spec) > Index: etc/etc.sgi/MAKEDEV.md > === > RCS file: /cvs/src/etc/etc.sgi/MAKEDEV.md,v > retrieving revision 1.54 > diff -u -p -r1.54 MAKEDEV.md > --- etc/etc.sgi/MAKEDEV.md6 Jul 2020 06:11:27 - 1.54 > +++ etc/etc.sgi/MAKEDEV.md28 Dec 2020 03:25:05 - > @@ -69,6 +69,7 @@ _DEV(uall) > _DEV(ugen, 63) > _DEV(uhid, 62) > _DEV(fido, 76) > +_DEV(ujoy, 78) > _DEV(ulpt, 64) > _DEV(usb, 61) > _TITLE(spec) > Index: etc/etc.sparc64/MAKEDEV.md > === > RCS file: /cvs/src/etc/etc.sparc64/MAKEDEV.md,v > retrieving revision 1.95 > diff -u -p -r1.95 MAKEDEV.md > --- etc/etc.sparc64/MAKEDEV.md22 Jul 2020 14:04:37 - 1.95 > +++ etc/etc.sparc64/MAKEDEV.md28 Dec 2020 03:25:05 - > @@ -104,6 +104,7 @@ _DEV(uall) > _DEV(ugen, 92) > _DEV(uhid, 91) > _DEV(fido, 137) > +_DEV(ujoy, 139) > _DEV(ulpt, 93) > _DEV(usb, 90) > _TITLE(spec) > Index: share/man/man4/Makefile > === > RCS file: /cvs/src/share/man/man4/Makefile,v > retrieving revision 1.790 > diff -u -p -r1.790 Makefile > --- share/man/man4/Makefile 6 Dec 2020 20:48:12 - 1.790 > +++ share/man/man4/Makefile 28 Dec 2020 03:25:31 - > @@ -84,7 +84,7 @@ MAN=aac.4 abcrtc.4 abl.4 ac97.4 acphy.4 > ubsec.4 ucom.4 uchcom.4 ucrcom.4 ucycom.4 ukspan.4 uslhcom.4 \ > udav.4 udcf.4 udl.4 udp.4 udsbr.4 \ > uftdi.4 ugen.4 ugl.4 ugold.4 uguru.4 uhci.4 uhid.4 uhidev.4 uipaq.4 \ > - uk.4 ukbd.4 \ > + ujoy.4 uk.4 ukbd.4 \ > ukphy.4 ulpt.4 umass.4 umb.4 umbg.4 umcs.4 umct.4 umidi.4 umodem.4 \ > ums.4 umsm.4 umstc.4 umt.4 unix.4 uonerng.4 uow.4 uoaklux.4 uoakrh.4 \ > uoakv.4 upd.4 upgt.4 upl.4 uplcom.4 ural.4 ure.4 url.4 urlphy.4 \ > Index: share/man/man4/uhidev.4 > ======= > RCS file: /cvs/src/share/man/man4/uhidev.4,v > retrieving revision 1.12 > diff -u -p -r1.12 uhidev.4 > --- share/man/man4/uhidev.4 21 Aug 2020 19:02:46 -
Re: clean /dev from /etc/daily ?
On Mon, Nov 23, 2020 at 03:25:34PM +0100, Otto Moerbeek wrote: > tOn Mon, Nov 23, 2020 at 01:53:01PM +0100, Solene Rapenne wrote: > > > A common mistake when using dd is to create a file in /dev which > > fills up the space of / and may stay silent until / gets filled up > > by something else that will fail. > > > > Would it be OK to add this in /etc/daily? > > > > find /dev -type f ! -name MAKEDEV -delete > > > > AFAIK /dev should have only MAKEDEV as a regular file. > > hier(7) says /dev only have block and character devices > > with the exception of MAKEDEV. > > > > reporting is good, but deleting not. > > -Otto ^^ I like this option a lot more, as users will get an opportunity to learn. -Bryan.
Re: clean /dev from /etc/daily ?
On Mon, Nov 23, 2020 at 03:19:24PM +0100, Mark Kettenis wrote: > > Date: Mon, 23 Nov 2020 13:53:01 +0100 > > From: Solene Rapenne > > > > A common mistake when using dd is to create a file in /dev which > > fills up the space of / and may stay silent until / gets filled up > > by something else that will fail. > > > > Would it be OK to add this in /etc/daily? > > > > find /dev -type f ! -name MAKEDEV -delete > > > > AFAIK /dev should have only MAKEDEV as a regular file. > > hier(7) says /dev only have block and character devices > > with the exception of MAKEDEV. > > Well, the man page is lying as /dev also contains: > > - sockets (/dev/log) > - symlinks To be fair, the -type f in solene@'s proposal would preserve all of those, and just cleanup stray normal files. This does seem to be mostly harmless, but I do wonder if silently cleaning up after a users mess is the way to go or not, and should this be done at boot instead, like the /tmp clearing? -Bryan.
Re: Ryzen 5800X hw.setperf vs hw.cpuspeed
On Fri, Nov 20, 2020 at 03:08:42PM +0100, Mark Kettenis wrote: > > Date: Fri, 20 Nov 2020 07:41:20 -0500 > > From: Bryan Steele > > > > On Fri, Nov 20, 2020 at 09:26:08AM +0100, Otto Moerbeek wrote: > > > Hi, > > > > > > I got a new Ryzen machine, dmesg below. What I'm observing might be a > > > issue with hw.setperf. > > > > > > On startsup it shows: > > > > > > hw.cpuspeed=3800 > > > hw.setperf=100 > > > > > > If I lower hw.setperf to zero, the new state is reflect immediately in > > > hw.cpuspeed: > > > > > > hw.cpuspeed=2200 > > > hw.setperf=0 > > > > > > And also sha256 -t becomes slower as expected. > > > > > > But If I raise hw.setperf to 100 I'm seeing: > > > > > > hw.cpuspeed=2200 > > > hw.setperf=100 > > > > > > and sha256 -t is still slow. Only after some time passes (lets say a > > > couple of tens of seconds) it does show: > > > > > > hw.cpuspeed=3800 > > > hw.setperf=100 > > > > > > and sha256 -t is fast again. > > > > > > This behaviour is different from my old machine, where setting > > > hs.setperf was reflected in hs.cpuspeed immediately both ways > > > > > > Any clue? > > > > > > -Otto > > > > Hey Otto, > > > > Nice machine! :-) > > > > I've seen this "sticking" issue before (as have others), but haven't > > been able to narrow it down unfortunately. I'm not sure if it's a > > bug in the k1x-pstate.c code I wrote, it's some undocumented new > > behaviour on newer Ryzen CPUs, or if a MI setperf change happened > > at some point that's unhandled.. > > > > At least on a desktop I'd suggest to leaved apmd(8) and not do any > > manual hw.setperf tweaking, you should have adequate cooling and the > > BIOS will automatically adjust the CPU fan to keep it so. I believe > > it will also allow it to more quickly move into CPB boost frequencies > > if left at P-state L0 (but don't quote me on that). > > I would expect this machine to use the acpucpu(4) setperf > implementation. Figuring out if that is indeed the case would > probably be step 1 in debugging this. I didn't realize there was a setperf implementation in acpicpu(4), k1x-pstate depends on acpicpu(4) to to gather PSS information, but otherwise writes the MSRs out itself rather than calling any ACPI methods. In identifycpu() we're just matching on family, e.g: if (ci->ci_family >= 0x10) setperf_setup = k1x_init; The Intel SpeedStep case is below and matches based on a CPUID flag, so I don't see when the acpucpu implementation would ever be chosen at on either.. -Bryan.
Re: Ryzen 5800X hw.setperf vs hw.cpuspeed
On Fri, Nov 20, 2020 at 08:21:41AM -0500, Bryan Steele wrote: > On Fri, Nov 20, 2020 at 01:13:02PM +, Stuart Henderson wrote: > > On 2020/11/20 07:41, Bryan Steele wrote: > > > At least on a desktop I'd suggest to leaved apmd(8) and not do any > > > manual hw.setperf tweaking, you should have adequate cooling > > > > the planet doesn't! :/ > > I'm only connered with burning your lap, not burning the earth. concerned^^
Re: Ryzen 5800X hw.setperf vs hw.cpuspeed
On Fri, Nov 20, 2020 at 01:13:02PM +, Stuart Henderson wrote: > On 2020/11/20 07:41, Bryan Steele wrote: > > At least on a desktop I'd suggest to leaved apmd(8) and not do any > > manual hw.setperf tweaking, you should have adequate cooling > > the planet doesn't! :/ I'm only connered with burning your lap, not burning the earth.
Re: Ryzen 5800X hw.setperf vs hw.cpuspeed
On Fri, Nov 20, 2020 at 01:13:02PM +, Stuart Henderson wrote: > On 2020/11/20 07:41, Bryan Steele wrote: > > At least on a desktop I'd suggest to leaved apmd(8) and not do any > > manual hw.setperf tweaking, you should have adequate cooling > > the planet doesn't! :/ hahaha
Re: Ryzen 5800X hw.setperf vs hw.cpuspeed
On Fri, Nov 20, 2020 at 07:41:22AM -0500, Bryan Steele wrote: > On Fri, Nov 20, 2020 at 09:26:08AM +0100, Otto Moerbeek wrote: > > Hi, > > > > I got a new Ryzen machine, dmesg below. What I'm observing might be a > > issue with hw.setperf. > > > > On startsup it shows: > > > > hw.cpuspeed=3800 > > hw.setperf=100 > > > > If I lower hw.setperf to zero, the new state is reflect immediately in > > hw.cpuspeed: > > > > hw.cpuspeed=2200 > > hw.setperf=0 > > > > And also sha256 -t becomes slower as expected. > > > > But If I raise hw.setperf to 100 I'm seeing: > > > > hw.cpuspeed=2200 > > hw.setperf=100 > > > > and sha256 -t is still slow. Only after some time passes (lets say a > > couple of tens of seconds) it does show: > > > > hw.cpuspeed=3800 > > hw.setperf=100 > > > > and sha256 -t is fast again. > > > > This behaviour is different from my old machine, where setting > > hs.setperf was reflected in hs.cpuspeed immediately both ways > > > > Any clue? > > > > -Otto > > Hey Otto, > > Nice machine! :-) > > I've seen this "sticking" issue before (as have others), but haven't > been able to narrow it down unfortunately. I'm not sure if it's a > bug in the k1x-pstate.c code I wrote, it's some undocumented new > behaviour on newer Ryzen CPUs, or if a MI setperf change happened > at some point that's unhandled.. > > At least on a desktop I'd suggest to leaved apmd(8) and not do any ... leave apmd disabled. :-) > manual hw.setperf tweaking, you should have adequate cooling and the > BIOS will automatically adjust the CPU fan to keep it so. I believe > it will also allow it to more quickly move into CPB boost frequencies > if left at P-state L0 (but don't quote me on that). > > -Bryan.
Re: Ryzen 5800X hw.setperf vs hw.cpuspeed
On Fri, Nov 20, 2020 at 09:26:08AM +0100, Otto Moerbeek wrote: > Hi, > > I got a new Ryzen machine, dmesg below. What I'm observing might be a > issue with hw.setperf. > > On startsup it shows: > > hw.cpuspeed=3800 > hw.setperf=100 > > If I lower hw.setperf to zero, the new state is reflect immediately in > hw.cpuspeed: > > hw.cpuspeed=2200 > hw.setperf=0 > > And also sha256 -t becomes slower as expected. > > But If I raise hw.setperf to 100 I'm seeing: > > hw.cpuspeed=2200 > hw.setperf=100 > > and sha256 -t is still slow. Only after some time passes (lets say a > couple of tens of seconds) it does show: > > hw.cpuspeed=3800 > hw.setperf=100 > > and sha256 -t is fast again. > > This behaviour is different from my old machine, where setting > hs.setperf was reflected in hs.cpuspeed immediately both ways > > Any clue? > > -Otto Hey Otto, Nice machine! :-) I've seen this "sticking" issue before (as have others), but haven't been able to narrow it down unfortunately. I'm not sure if it's a bug in the k1x-pstate.c code I wrote, it's some undocumented new behaviour on newer Ryzen CPUs, or if a MI setperf change happened at some point that's unhandled.. At least on a desktop I'd suggest to leaved apmd(8) and not do any manual hw.setperf tweaking, you should have adequate cooling and the BIOS will automatically adjust the CPU fan to keep it so. I believe it will also allow it to more quickly move into CPB boost frequencies if left at P-state L0 (but don't quote me on that). -Bryan.
Re: Xterm close delay when child processes exist
On Sun, Nov 08, 2020 at 05:16:55PM +0100, Stefan Hagen wrote:
> Hello,
>
> xterm has an annoying delay when it's being close by the window manager
> when child processes exist.
>
> Test 1:
> xterm -e "sh"
>
> Now hit the "X" button or whatever your window manager provides to send a
> window delete event. Xterm should close almost instantly.
>
> Test 2:
> xterm -e "sh -c sh"
>
> You can execute this, or just open xterm and open another shell inside.
> Now close it again. In this situation it takes about 1 second to close.
>
> Window deletion triggers do_hangup(), which invokes a kill(-pid, SIGHUP)
> and then waits around one second before it quits. I couldn't figure out
> what it does in this time. But ktrace hints that it ran into a Sleep()
> statement because the delay is right after a select and fd_set, which is
> only called in Sleep().
>
> I looked and the xterm menu Quit function and found that it does something
> slightly different than than window delete. It also sends kill(-pid,SIGHUP),
> but then immediately exits.
>
> Handle_send_signal and Cleanup are doing the same thing, except Cleanup
> calls exit right afterwards.
>
> My naive assumtion is, that this would also be the right behavior on
> window deletion.
>
This would be better off sent upstream, not maintained locally in
OpenBSD.
https://invisible-island.net/xterm/xterm.faq.html#report_bugs
> Index: app/xterm/menu.c
> ===
> RCS file: /cvs/xenocara/app/xterm/menu.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 menu.c
> --- app/xterm/menu.c 20 Jan 2020 21:03:35 - 1.30
> +++ app/xterm/menu.c 8 Nov 2020 15:49:08 -
> @@ -1422,7 +1422,7 @@ do_hangup(Widget gw,
> XtPointer closure GCC_UNUSED,
> XtPointer data GCC_UNUSED)
> {
> -handle_send_signal(gw, SIGHUP);
> +Cleanup(SIGHUP);
> }
>
> /* ARGSUSED */
>
>
Re: Happy 25th Birthday OpenBSD!
On Sun, Oct 18, 2020 at 09:44:52AM -0600, Bob Beck wrote: > > Yeah, it's just a number. > > But it's been a pretty wild ride. Thanks everyone for 25 years. > > -Bob > > > > Happy 25th everybody! \o/ -Bryan.
Re: current.html: i586 requirement for i386 architecture
On Fri, Aug 07, 2020 at 03:49:32PM +0200, Solene Rapenne wrote: > Now that i386 platform requires i586 CPU, I guess we should mention > it in current.html (the page i386.html should be updated accordingly > at 6.8 release) > > Index: current.html > === > RCS file: /home/reposync/www/faq/current.html,v > retrieving revision 1.1048 > diff -u -p -r1.1048 current.html > --- current.html 11 Jul 2020 16:53:47 - 1.1048 > +++ current.html 7 Aug 2020 13:43:25 - > @@ -57,6 +57,16 @@ use a snapshot to recover. > Most of these changes will have to be performed as root. > > > +2020/08/03 - i386 requirements raised to i586 > + > +Due to llvm upgrade to version 10 in base system, the minimal > +CPU instruction sets requirements for i386 architecture has been > +raised from i386 to i586. I can't speak as to whether or not this is warrented or not, but this part is at least wrong. i386 CPU support was dropped years ago in the kernel, only 486-class processors and higher (with a math-coprocessor) are supported. The real hint here might be that cpu0: must contain "CX8" to reliably work from now on, which indicates support for the CMPXCHG8 instruction. > +In concrete terms for Intel and AMD CPUs, Intel Pentium and AMD K5 > +are now the minimal requirements for i386 architecture. > + > + > 2020/05/29 - [ports] net/isc-bind example files > dropped > > Outdated example configuration (including some "standard" zone files and > >
Re: exFAT support
On Thu, Aug 06, 2020 at 02:16:11PM -0700, jo...@armadilloaerospace.com wrote: > I tried to mount a 12TB USB drive, and was getting an "Inappropriate > file type or format" error. > > It turned out to be due to exFAT formatting, but it took me some > investigating to figure that out. Would it be reasonable to have the > kernel print a more informative warning like "exFAT filesystem not > supported" when you try to mount it with mount_msdos, or are additional > kernel prints considered bad form? Yes, EFTYPE error is pretty much the best you can do. mount_msdos is only for FAT12/16/32, in the same way you can't mount FFS with it or NTFS. More informative errors here would require writing a lot more parsing code in the kernel, which is already a sensitive area. > With Microsoft's release of the spec last year, is the path open for > kernel support now, when someone gets around to it? I don't know the details, but I believe one issue with exFAT has been Microsoft and software patents, not just available documentation. Linux may eventually get a kernel implementation but I'm not sure that helps us. https://www.zdnet.com/article/microsoft-readies-exfat-patents-for-linux-and-open-source/ Maybe that's not important, and it's just a lack of both interest and capable developers hacking on new filesystems. > I have seen some comments about the reliability of exfat-fuse, will I > be sad if I try to use it for something important? I mean, can mount the volume read-only if you don't intend to write. If interoperability isn't a concern, choosing a different filesystem (e.g. FFS2) might be alternative option. Hope that helps, -Bryan.
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 01:44:33PM -0400, Johan Huldtgren wrote:
> hello,
>
> On 2020-07-28 11:12, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew
> > >
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis
> > > > >
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > diff tries to support machines with firmware that implements this. If
> > > > > you see something like:
> > > > >
> > > > > "ACPI0007" at acpi0 not configured
> > > > >
> > > > > please try the following diff and report back with an updated dmesg.
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Mark
> > > >
> > > > And now with the right diff...
> > >
> > > On a dell r6415, it looks like this:
> > >
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > >
> > > which I guess means aml_copyvalue() needs to learn how to copy
> > > AML_OBJTYPE_DEVICE.
> >
> > Yes. It is not immediately obvious how this should work. Do we need
> > to copy the aml_node pointer or not? We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > carry any additional information. So we end up with just an empty
> > case to avoid the warning.
> >
> > Does this work on the Dell machines?
> >
> >
> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > + case AML_OBJTYPE_DEVICE:
> > + break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
> >
Mark, This one below looks like it's attaching on both Processor() nodes
and "ACPI0007", I'm assuming we don't want to do that?
> before diffs I see:
>
> "ACPI0004" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0004" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
>
> after applying both diffs I see that as
>
> "ACPI0004" at acpi0 not configured
> acpicpu24 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu25 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu26 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu27 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu28 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu29 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu30 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu31 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu32 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu33 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu34 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu35 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> "ACPI0004" at acpi0 not configured
> acpicpu36 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu37 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu38 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu39 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu40 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu41 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu42 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu43 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu44 at acpi0: C2(350@41 mwait.3@0x20),
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 01:09:51PM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 11:16:56 +0100
> > From: Jason McIntyre
> >
> > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > From: Jonathan Matthew
> > > >
> > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > From: Mark Kettenis
> > > > > >
> > > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout
> > > > > > of
> > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > > diff tries to support machines with firmware that implements this.
> > > > > > If
> > > > > > you see something like:
> > > > > >
> > > > > > "ACPI0007" at acpi0 not configured
> > > > > >
> > > > > > please try the following diff and report back with an updated dmesg.
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Mark
> > > > >
> > > > > And now with the right diff...
> > > >
> > > > On a dell r6415, it looks like this:
> > > >
> > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > all the way up to
> > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > >
> > > > which I guess means aml_copyvalue() needs to learn how to copy
> > > > AML_OBJTYPE_DEVICE.
> > >
> > > Yes. It is not immediately obvious how this should work. Do we need
> > > to copy the aml_node pointer or not? We don't do that for
> > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > > carry any additional information. So we end up with just an empty
> > > case to avoid the warning.
> > >
> > > Does this work on the Dell machines?
> > >
> > >
> > > Index: dev/acpi/dsdt.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > retrieving revision 1.252
> > > diff -u -p -r1.252 dsdt.c
> > > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > > lhs->v_objref = rhs->v_objref;
> > > aml_addref(lhs->v_objref.ref, "");
> > > break;
> > > + case AML_OBJTYPE_DEVICE:
> > > + break;
> > > default:
> > > printf("copyvalue: %x", rhs->type);
> > > break;
> > >
> >
> > morning. it displays this here:
> >
> > acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu8 at acpi0: no cpu matching ACPI ID 8
> > acpicpu9 at acpi0: no cpu matching ACPI ID 9
> > acpicpu10 at acpi0: no cpu matching ACPI ID 10
> > acpicpu11 at acpi0: no cpu matching ACPI ID 11
> > acpicpu12 at acpi0: no cpu matching ACPI ID 12
> > acpicpu13 at acpi0: no cpu matching ACPI ID 13
> > acpicpu14 at acpi0: no cpu matching ACPI ID 14
> > acpicpu15 at acpi0: no cpu matching ACPI ID 15
>
> Excellent!
>
> We may want to do something about those "no cpu matching ACPU ID XX"
> messages at some point. But that's a diff for another day.
>
> So ok's for both diffs are welcome.
reads fine and clearly improves things for limited function ACPI
systems.
ok brynet@
> Cheers,
>
> Mark
>
> P.S. I've also established that that the EC-related messages are
> indeed harmless and a result of sloppy BIOS writers.
>
> > OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
> > jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > real mem = 7895654400 (7529MB)
> > avail mem = 7641284608 (7287MB)
> > random: good seed from bootblocks
> > mpath0 at root
> > scsibus0 at mpath0: 256 targets
> > mainbus0 at root
> > bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
> > bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
> > bios0: Dell Inc. Inspiron 5505
> > acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_
> >
> > acpi0: sleep states S0 S4 S5
> > acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC
> > MCFG SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT
> > SSDT SSDT SSDT SSDT SSDT SSDT BGRT
> > acpi0: wakeup devices GP17(S4)
> > acpitimer0 at acpi0: 3579545
Re: acpicpu(4) and ACPI0007
On Mon, Jul 27, 2020 at 05:02:41PM +0200, Mark Kettenis wrote:
> Recent ACPI versions have deprecated "Processor()" nodes in favout of
> "Device()" nodes with a _HID() method that returns "ACPI0007". This
> diff tries to support machines with firmware that implements this. If
> you see something like:
>
> "ACPI0007" at acpi0 not configured
>
> please try the following diff and report back with an updated dmesg.
>
> Cheers,
>
> Mark
>
Wrong diff?
>
> Index: dev/acpi/acpi.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.384
> diff -u -p -r1.384 acpi.c
> --- dev/acpi/acpi.c 11 May 2020 17:57:17 - 1.384
> +++ dev/acpi/acpi.c 13 May 2020 18:44:32 -
> @@ -72,6 +72,7 @@ int acpi_debug = 16;
>
> int acpi_poll_enabled;
> int acpi_hasprocfvs;
> +int acpi_haspci;
>
> #define ACPIEN_RETRIES 15
>
> Index: dev/acpi/acpivar.h
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
> retrieving revision 1.108
> diff -u -p -r1.108 acpivar.h
> --- dev/acpi/acpivar.h8 May 2020 11:18:01 - 1.108
> +++ dev/acpi/acpivar.h13 May 2020 18:44:32 -
> @@ -43,6 +43,7 @@ extern int acpi_debug;
> #endif
>
> extern int acpi_hasprocfvs;
> +extern int acpi_haspci;
>
> struct klist;
> struct acpiec_softc;
> Index: arch/amd64/amd64/mainbus.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/mainbus.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 mainbus.c
> --- arch/amd64/amd64/mainbus.c7 Sep 2019 13:46:19 - 1.49
> +++ arch/amd64/amd64/mainbus.c13 May 2020 18:44:32 -
> @@ -231,6 +231,13 @@ mainbus_attach(struct device *parent, st
> #endif
>
> #if NPCI > 0
> +#if NACPI > 0
> + if (acpi_haspci) {
> + extern void acpipci_attach_busses(struct device *);
> +
> + acpipci_attach_busses(self);
> + } else
> +#endif
> {
> pci_init_extents();
>
> @@ -245,9 +252,6 @@ mainbus_attach(struct device *parent, st
> mba.mba_pba.pba_domain = pci_ndomains++;
> mba.mba_pba.pba_bus = 0;
> config_found(self, _pba, mainbus_print);
> -#if NACPI > 0
> - acpi_pciroots_attach(self, _pba, mainbus_print);
> -#endif
> }
> #endif
>
> Index: arch/amd64/conf/RAMDISK
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK,v
> retrieving revision 1.77
> diff -u -p -r1.77 RAMDISK
> --- arch/amd64/conf/RAMDISK 5 Mar 2020 16:36:30 - 1.77
> +++ arch/amd64/conf/RAMDISK 13 May 2020 18:44:32 -
> @@ -30,6 +30,7 @@ acpi0 at bios?
> #acpicpu*at acpi?
> acpicmos*at acpi?
> acpiec* at acpi?
> +acpipci* at acpi?
> acpiprt* at acpi?
> acpimadt0at acpi?
> #acpitz* at acpi?
> Index: arch/amd64/conf/RAMDISK_CD
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK_CD,v
> retrieving revision 1.188
> diff -u -p -r1.188 RAMDISK_CD
> --- arch/amd64/conf/RAMDISK_CD15 Feb 2020 08:49:11 - 1.188
> +++ arch/amd64/conf/RAMDISK_CD13 May 2020 18:44:32 -
> @@ -37,6 +37,7 @@ acpi0 at bios?
> #acpicpu*at acpi?
> acpicmos*at acpi?
> acpiec* at acpi?
> +acpipci* at acpi?
> acpiprt* at acpi?
> acpimadt0at acpi?
> #acpitz* at acpi?
> Index: arch/amd64/pci/acpipci.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/pci/acpipci.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 acpipci.c
> --- arch/amd64/pci/acpipci.c 7 Sep 2019 13:46:19 - 1.3
> +++ arch/amd64/pci/acpipci.c 13 May 2020 18:44:32 -
> @@ -53,6 +53,19 @@ struct acpipci_softc {
> struct device sc_dev;
> struct acpi_softc *sc_acpi;
> struct aml_node *sc_node;
> +
> + bus_space_tag_t sc_iot;
> + bus_space_tag_t sc_memt;
> + bus_dma_tag_t sc_dmat;
> +
> + struct extent *sc_busex;
> + struct extent *sc_memex;
> + struct extent *sc_ioex;
> + charsc_busex_name[32];
> + charsc_ioex_name[32];
> + charsc_memex_name[32];
> + int sc_bus;
> + uint32_tsc_seg;
> };
>
> int acpipci_match(struct device *, void *, void *);
> @@ -72,6 +85,11 @@ const char *acpipci_hids[] = {
> NULL
> };
>
> +void acpipci_attach_deferred(struct device *);
> +int acpipci_print(void *, const char *);
> +int acpipci_parse_resources(int, union acpi_resource *, void *);
> +void acpipci_osc(struct acpipci_softc *);
> +
> int
> acpipci_match(struct device *parent, void *match, void *aux)
> {
> @@ -86,15 +104,225 @@ acpipci_attach(struct device *parent, st
> {
>
Re: A concerning commit which breaks compatibility
On Thu, Jul 23, 2020 at 09:54:56PM +, goldeneagle96 wrote: > Hello OpenBSD devs. It has come to my attention that a mysterious commit > , unlogged by CVS, has appeared. This commit changes language, breaking > compatibility on header and source files. > Thankfully, it was logged by the Github mirror. > The commit's author is the Github username "djmdjm", and the one who > okayed it was "markus@". > Please, I ask of you and specially of Theo to look at this strange > commit, and decide what to do about it. > Its link is > https://github.com/openbsd/src/commit/5bde2954c180034a27b079acaff46073dc75139b > cc @misc @tech There is nothing "mysterious" about this commit, and the GitHub repo is a read-only mirror, nobody commits to it. https://marc.info/?l=openbsd-cvs=159399360827650=2 > .. breaking compatibility on header and source files. OpenSSH installs no libraries or headers, and has no public API. -Bryan.
Re: SSE in kernel?
On Tue, Jun 23, 2020 at 01:03:18PM +0200, Patrick Wildt wrote: > On Tue, Jun 23, 2020 at 06:51:20AM -0400, Bryan Steele wrote: > > On Mon, Jun 22, 2020 at 11:10:10PM -0700, jo...@armadilloaerospace.com > > wrote: > > > Are SSE instructions allowed in the AMD64 kernel? Is #ifdef __SSE__ > > > a sufficient guard? > > > > > > I have a rasops32 putchar with SSE that is 2x faster. > > > > No, in general you cannot using FP instructions in the kernel, also the > > kernel is often compiled with -msoft-float on platforms that support it. > > Exceptions are being made for amdgpu drm, where some of the files are > compiled with sse enabled, though the code is guarded with > fpu_kernel_enter(). DC_FP_START() and DC_FP_END() to be precise, which > are macros pointing to the kernel functions. ^ patrick got there first. :-)
Re: SSE in kernel?
On Tue, Jun 23, 2020 at 06:51:22AM -0400, Bryan Steele wrote: > On Mon, Jun 22, 2020 at 11:10:10PM -0700, jo...@armadilloaerospace.com wrote: > > Are SSE instructions allowed in the AMD64 kernel? Is #ifdef __SSE__ > > a sufficient guard? > > > > I have a rasops32 putchar with SSE that is 2x faster. > > No, in general you cannot using FP instructions in the kernel, also the > kernel is often compiled with -msoft-float on platforms that support it. More specifically using the FP instructions the kernel incurs a performance penalty from having to save and restore the FP registers. There are some helper functions for that in the very limited cases where it's done, see fpu_kernel_enter/exit() in arch/amd64/amd64/fpu.c. But these are i386/amd64-only.
Re: SSE in kernel?
On Mon, Jun 22, 2020 at 11:10:10PM -0700, jo...@armadilloaerospace.com wrote: > Are SSE instructions allowed in the AMD64 kernel? Is #ifdef __SSE__ > a sufficient guard? > > I have a rasops32 putchar with SSE that is 2x faster. No, in general you cannot using FP instructions in the kernel, also the kernel is often compiled with -msoft-float on platforms that support it.
Re: Good CoreBoot system?
On Sat, Jun 13, 2020 at 10:41:06PM +0200, Lorenz Troiza wrote: > Am 13.06.2020 um 22:27 schrieb jo...@armadilloaerospace.com: > > What would be a good system with CoreBoot to get for OpenBSD testing? > > > > It looks like there are some edge cases where the CoreBoot framebuffer > > support won't behave the same as the EFI framebuffer. > > > > > I'm running OpenBSD on a NRG-Systems apu4d2 board. This board has > coreboot by default. I haven't yet experienced any issues. > > Cheers > > Lorenz Those systems don't have a graphics framebuffer, which is the entire point of John's email.
Re: Avoid offline cpu in automatic frequency scheduling
On Thu, May 28, 2020 at 04:29:19PM +0200, Solene Rapenne wrote:
> the macro CPU_INFO_FOREACH loop over every CPU but the frequency
> algorithm will raise frequency if one cpu usage goes over a threshold
> but also if the sum of cpu usage goes over another threshold.
>
> In the current case, if you have offline cpu (because of hw.smt=0), the
> total will still sum all the idle cpu and it's unlikely that the total
> threshold is ever reached before the per cpu one.
>
> so, this diff skip offline cpus in the loop (robert@ gave me the big
> clue to use cpu_is_online in the loop)
Thanks for finding this solene! Nice work.
I've been aware of some issues on AMD with the automatic frequency
scaling and haven't been able to narrow them down. This may not solve
all of them, but does appear to improve things on Ryzen CPUs which have
many cores/threads and no BIOS knob to disable SMT.
Assuming others agree with the approach..
ok brynet@
> Index: sched_bsd.c
> ===
> RCS file: /cvs/src/sys/kern/sched_bsd.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 sched_bsd.c
> --- sched_bsd.c 30 Jan 2020 08:51:27 - 1.62
> +++ sched_bsd.c 28 May 2020 14:21:25 -
> @@ -576,6 +576,8 @@ setperf_auto(void *v)
> j = 0;
> speedup = 0;
> CPU_INFO_FOREACH(cii, ci) {
> + if (!cpu_is_online(ci))
> + continue;
> total = 0;
> for (i = 0; i < CPUSTATES; i++) {
> total += ci->ci_schedstate.spc_cp_time[i];
>
>
Re: pledge(2) sndioctl(1)
On Thu, May 21, 2020 at 11:07:39PM +0100, Ricardo Mestre wrote:
> Hi,
>
> After the handle sioctl_hdl `hdl' is opened (which in itself requires rw fs
> access and opening an unix socket) then all operations happen over that handle
> so the program may be restricted to only "stdio".
>
> All options were tested successfully, including the examples from the manpage
> plus tweaking the audio from an app ($MYBROWSER).
>
> Comments? OK?
Works for me.
ok brynet@
> Index: sndioctl.c
> ===
> RCS file: /cvs/src/usr.bin/sndioctl/sndioctl.c,v
> retrieving revision 1.10
> diff -u -p -u -r1.10 sndioctl.c
> --- sndioctl.c17 May 2020 05:39:32 - 1.10
> +++ sndioctl.c21 May 2020 22:04:58 -
> @@ -948,6 +948,13 @@ main(int argc, char **argv)
> fprintf(stderr, "%s: can't open control device\n", devname);
> exit(1);
> }
> +
> + if (pledge("stdio", NULL) == -1) {
> + fprintf(stderr, "%s: pledge: %s\n", getprogname(),
> + strerror(errno));
> + exit(1);
> + }
> +
> if (!sioctl_ondesc(hdl, ondesc, NULL)) {
> fprintf(stderr, "%s: can't get device description\n", devname);
> exit(1);
>
>
Re: sdmmc(4): use DMA for all commands if supported
On Sat, Apr 18, 2020 at 03:07:28PM +0200, Tobias Heider wrote: > Hi, > > the attached diff allows sdmmc(4) to use DMA for all commands instead of just > mem_read and mem_write. > > There were problems in the past with some controllers not liking small DMA > transfers, so it would be nice to get this tested thorougly on different > hardware. So far I have successfully tested it on a Raspberry Pi 3 B. This a spoiler or something? I don't have working sdmmc(4) on my RPi3 B.
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 04:56:31PM +0200, Martijn van Duren wrote:
> This came up during u2k20 while discussing tempfiles for gotweb inside a
> chroot. At the moment we don't include it by default and ports have to
> create it themselves. Since I assume we want web applications to run
> inside a /var/www chroot as much as possible and even some libc
> functions depend on /tmp being available I'd argue we should include it
> by default.
WIth FastCGI, perhaps I'm confused, but why do web applications need to
be inside the /var/www chroot? Can't they be anywhere, or even have a
seperate chroot directory? Should we be handling things things that
are not in base?
> I also choose to make the directory 1777, similar to a normal /tmp,
> since both multiple slowcgi or php-fpm pools can run simultaneously
> under different users.
>
> The cleanup functions don't reflect the current /tmp cleanup style, but
> we can move the existing find statements to -delete in a separate patch.
>
> I already had some positive feedback during u2k20 on the concept.
> OK?
>
> martijn@
>
> Index: etc//daily
> ===
> RCS file: /cvs/src/etc/daily,v
> retrieving revision 1.93
> diff -u -p -r1.93 daily
> --- etc//daily9 Sep 2019 20:02:26 - 1.93
> +++ etc//daily7 Apr 2020 14:37:15 -
> @@ -55,6 +55,11 @@ if [ -d /tmp -a ! -L /tmp ]; then
> ! -path ./.ICE-unix ! -name . \
> -execdir rmdir -- {} \; >/dev/null 2>&1; }
> fi
> +if [ -d /var/www/tmp -a ! -L /var/www/tmp ]; then
> + cd /var/www/tmp && {
> + find -x . -type f -atime +7 -delete 2>/dev/null
> + find -x . -type d -empty -delete 2>/dev/null
> +fi
>
> # Additional junk directory cleanup would go like this:
> #if [ -d /scratch -a ! -L /scratch ]; then
> Index: etc//rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.543
> diff -u -p -r1.543 rc
> --- etc//rc 24 Jan 2020 06:17:37 - 1.543
> +++ etc//rc 7 Apr 2020 14:37:15 -
> @@ -532,7 +532,7 @@ if [[ -f /etc/ptmp ]]; then
> 'password file may be incorrect -- /etc/ptmp exists'
> fi
>
> -echo clearing /tmp
> +echo clearing temporary directories
>
> # Prune quickly with one rm, then use find to clean up /tmp/[lqv]*
> # (not needed with mfs /tmp, but doesn't hurt there...).
> @@ -540,6 +540,7 @@ echo clearing /tmp
> (cd /tmp &&
> find . -maxdepth 1 ! -name . ! -name lost+found ! -name quota.user \
> ! -name quota.group ! -name vi.recover -execdir rm -rf -- {} \;)
> +(cd /var/www/tmp && find . -x -delete)
>
> # Create Unix sockets directories for X if needed and make sure they have
> # correct permissions.
> Index: etc//mtree/4.4BSD.dist
> ===
> RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> retrieving revision 1.314
> diff -u -p -r1.314 4.4BSD.dist
> --- etc//mtree/4.4BSD.dist29 Nov 2019 03:28:20 - 1.314
> +++ etc//mtree/4.4BSD.dist7 Apr 2020 14:37:15 -
> @@ -749,6 +749,7 @@ var
> ..
> run type=dir uname=root gname=daemon mode=755
> ..
> +tmp type=dir uname=root gname=wheel mode=01777
> ..
>
> # ./var/audit
>
>
Re: arm64 rpi3b install method
On Sat, Mar 07, 2020 at 04:50:52PM +, Stuart Henderson wrote: > On 2020/03/07 15:41, Jonathan Gray wrote: > > On Fri, Mar 06, 2020 at 11:29:57PM +, Stuart Henderson wrote: > > > I've finally managed to get openbsd installed on an rpi3b (need > > > something to run signify/pkg_sign and this is what I have). Thought I'd > > > write up the install method because there are no useful docs at the > > > moment and it's a bit fiddly. (Note that only rpi3b works - 3b+ has no > > > network/usb, the 32-bit ones are unsupported, 4 is unsupported). > > > > > > - boot linux, set the otp bit to permanently enable booting from usb (set > > > "program_usb_boot_mode=1" in /boot/config.txt and reboot) > > > > You don't need to boot linux for that. Just boot with rpi firmware. > > 3b+ can boot off usb by default. > > > > And if you don't want to blow the fuse the sd card can be left in and > > boot_targets set in the U-Boot environment as mentioned on arm64.html. > > I had to borrow the SD card from something else so I'd prefer to have > it back than not blow the fuse :) > > > > > > > - write miniroot64.fs to an SD card (I tried 6.3 up to 6.6 and -current, > > > that's the only one where I get any console output) > > > > Are you using pins 8 (tx) and 10 (rx)? > > > > With the Ethernet port facing towards you numbering is > > > > 1 2 > > 3 4 > > 5 6 > > 7 8 > > 9 10 > > > > After 6.6 the uart is now the more capable pl011 by default. > > Yes, I'm using 8/10. > > 6.4 is the only one from 6.3 to -current where, when written to the SD card, > I had any output. Weird. I wrote -current miniroot66.fs to a SD and it works in the SD and also USB via a USB->SD adapter (I had blown the usbboot fuse previously). But the firmware is very finicky on booting from USB devices, I have several USB flash sticks that simply don't work. Installed without issue, don't capture serial output unfortunately. OpenBSD 6.6-current (GENERIC.MP) #487: Fri Mar 6 11:52:20 MST 2020 dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP real mem = 958996480 (914MB) avail mem = 899473408 (857MB) mainbus0 at root: Raspberry Pi 3 Model B Rev 1.2 cpu0 at mainbus0 mpidr 0: ARM Cortex-A53 r0p4 cpu0: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache cpu0: 512KB 64b/line 16-way L2 cache efi0 at mainbus0: UEFI 2.8 efi0: Das U-Boot rev 0x20191000 apm0 at mainbus0 simplefb0 at mainbus0: 656x416, 32bpp wsdisplay0 at simplefb0 mux 1 wsdisplay0: screen 0-5 added (std, vt100 emulation) "system" at mainbus0 not configured "axi" at mainbus0 not configured "thermal-zones" at mainbus0 not configured simplebus0 at mainbus0: "soc" "dma" at simplebus0 not configured bcmdog0 at simplebus0 "cprman" at simplebus0 not configured bcmrng0 at simplebus0 "mailbox" at simplebus0 not configured "gpio" at simplebus0 not configured pluart0 at simplebus0: console "mmc" at simplebus0 not configured "dsi" at simplebus0 not configured bcmaux0 at simplebus0 dwctwo0 at simplebus0 bcmintc0 at simplebus0 bcmtemp0 at simplebus0 "local_intc" at simplebus0 not configured "mmcnr" at simplebus0 not configured simplebus1 at simplebus0: "firmware" "expgpio" at simplebus1 not configured "power" at simplebus0 not configured "mailbox" at simplebus0 not configured "gpiomem" at simplebus0 not configured "fb" at simplebus0 not configured "vcsm" at simplebus0 not configured "virtgpio" at simplebus0 not configured simplebus2 at mainbus0: "clocks" "clock" at simplebus2 not configured "clock" at simplebus2 not configured "phy" at mainbus0 not configured "arm-pmu" at mainbus0 not configured agtimer0 at mainbus0: tick rate 19200 KHz "__overrides__" at mainbus0 not configured "leds" at mainbus0 not configured "fixedregulator_3v3" at mainbus0 not configured "fixedregulator_5v0" at mainbus0 not configured "__symbols__" at mainbus0 not configured cpu1 at mainbus0 mpidr 1: ARM Cortex-A53 r0p4 cpu1: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache cpu1: 512KB 64b/line 16-way L2 cache cpu2 at mainbus0 mpidr 2: ARM Cortex-A53 r0p4 cpu2: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache cpu2: 512KB 64b/line 16-way L2 cache cpu3 at mainbus0 mpidr 3: ARM Cortex-A53 r0p4 cpu3: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache cpu3: 512KB 64b/line 16-way L2 cache usb0 at dwctwo0: USB revision 2.0 uhub0 at usb0 configuration 1 interface 0 "Broadcom DWC2 root hub" rev 2.00/1.00 addr 1 uhub1 at uhub0 port 1 configuration 1 interface 0 "Standard Microsystems product 0x9514" rev 2.00/2.00 addr 2 smsc0 at uhub1 port 1 configuration 1 interface 0 "Standard Microsystems SMSC9512/14" rev 2.00/2.00 addr 3 smsc0: address b8:27:eb:bd:e8:32 ukphy0 at smsc0 phy 1: Generic IEEE 802.3u media interface, rev. 3: OUI 0x0001f0, model 0x000c umass0 at uhub1 port 3 configuration 1 interface 0 "vendor 0x1908 product 0x0226" rev 2.00/1.11 addr 4 umass0: using SCSI over Bulk-Only scsibus0 at umass0: 2 targets, initiator 0 sd0 at
Re: cwm: remove menu-ssh
On Wed, Jan 22, 2020 at 03:15:37PM -0500, Okan Demirmen wrote:
> Hi,
>
> I think we've (or at least I have) mused about this for a while; a
> recent mail reminded me that this feature should go - a window manager
> doesn't need to parse the ssh known_hosts file for a menu; there are
> better tools for this.
>
> Remove menu-ssh.
>
> okay?
I remember agreeing last time. Had this in .cwmrc for awhile. :-)
unbind-key M-period # ssh menu
ok brynet@
> Index: calmwm.h
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
> retrieving revision 1.372
> diff -u -p -r1.372 calmwm.h
> --- calmwm.h 22 Jan 2020 19:58:35 - 1.372
> +++ calmwm.h 22 Jan 2020 20:09:13 -
> @@ -304,7 +304,6 @@ struct conf {
> int xrandr;
> int xrandr_event_base;
> char*conf_file;
> - char*known_hosts;
> char*wm_argv;
> int debug;
> };
> @@ -517,7 +516,6 @@ void kbfunc_menu_cmd(void *, struct
> c
> void kbfunc_menu_group(void *, struct cargs *);
> void kbfunc_menu_wm(void *, struct cargs *);
> void kbfunc_menu_exec(void *, struct cargs *);
> -void kbfunc_menu_ssh(void *, struct cargs *);
> void kbfunc_client_menu_label(void *, struct cargs *);
> void kbfunc_exec_cmd(void *, struct cargs *);
> void kbfunc_exec_lock(void *, struct cargs *);
> Index: conf.c
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
> retrieving revision 1.249
> diff -u -p -r1.249 conf.c
> --- conf.c7 Mar 2019 12:54:21 - 1.249
> +++ conf.c22 Jan 2020 20:09:24 -
> @@ -179,7 +179,6 @@ static const struct {
>
> { FUNC_SC(menu-cmd, menu_cmd, 0) },
> { FUNC_SC(menu-group, menu_group, 0) },
> - { FUNC_SC(menu-ssh, menu_ssh, 0) },
> { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) },
> { FUNC_SC(menu-window-hidden, menu_client, CWM_MENU_WINDOW_HIDDEN) },
> { FUNC_SC(menu-exec, menu_exec, 0) },
> @@ -210,7 +209,6 @@ static const struct {
> { "CM-Delete", "lock" },
> { "M-question", "menu-exec" },
> { "CM-w", "menu-exec-wm" },
> - { "M-period", "menu-ssh" },
> { "M-Return", "window-hide" },
> { "M-Down", "window-lower" },
> { "M-Up", "window-raise" },
> @@ -316,7 +314,6 @@ conf_init(struct conf *c)
> home = "/";
> }
> xasprintf(>conf_file, "%s/%s", home, ".cwmrc");
> - xasprintf(>known_hosts, "%s/%s", home, ".ssh/known_hosts");
> }
>
> void
> @@ -363,7 +360,6 @@ conf_clear(struct conf *c)
> free(c->color[i]);
>
> free(c->conf_file);
> - free(c->known_hosts);
> free(c->font);
> free(c->wmname);
> }
> Index: cwm.1
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v
> retrieving revision 1.65
> diff -u -p -r1.65 cwm.1
> --- cwm.1 9 Jul 2019 21:38:44 - 1.65
> +++ cwm.1 22 Jan 2020 20:08:19 -
> @@ -140,15 +140,6 @@ Resize window by a large amount; see
> Spawn
> .Dq exec program
> dialog.
> -.It Ic M-period
> -Spawn
> -.Dq ssh to
> -dialog.
> -This parses
> -.Pa $HOME/.ssh/known_hosts
> -to provide host auto-completion.
> -.Xr ssh 1
> -will be executed via the configured terminal emulator.
> .It Ic CM-w
> Spawn
> .Dq exec WindowManager
> Index: cwmrc.5
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
> retrieving revision 1.73
> diff -u -p -r1.73 cwmrc.5
> --- cwmrc.5 2 Jul 2019 23:37:47 - 1.73
> +++ cwmrc.5 22 Jan 2020 20:07:57 -
> @@ -280,10 +280,6 @@ menu.
> Launch
> .Dq exec WindowManager
> menu.
> -.It menu-ssh
> -Launch
> -.Dq ssh
> -menu.
> .It group-toggle-[n]
> Toggle visibility of group n, where n is 1-9.
> .It group-only-[n]
> Index: kbfunc.c
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
> retrieving revision 1.167
> diff -u -p -r1.167 kbfunc.c
> --- kbfunc.c 21 Jan 2020 15:50:03 - 1.167
> +++ kbfunc.c 22 Jan 2020 20:09:03 -
> @@ -647,72 +647,6 @@ out:
> }
>
> void
> -kbfunc_menu_ssh(void *ctx, struct cargs *cargs)
> -{
> - struct screen_ctx *sc = ctx;
> - struct cmd_ctx *cmd;
> - struct menu *mi;
> - struct menu_qmenuq;
> - FILE*fp;
> - char*buf, *lbuf, *p;
> - char hostbuf[HOST_NAME_MAX+1];
> - char path[PATH_MAX];
> - int l;
> - size_t
Re: cdio(1): remove CDDB support
On Sat, Dec 28, 2019 at 08:03:17PM -0500, Bryan Steele wrote: > On Sat, Dec 28, 2019 at 07:48:47PM -0500, Bryan Steele wrote: > > With FreeDB announcing[0] that the service will be shutting down as of > > March 31st of 2020, and the only other alternative (MusicBrainz) already > > having shutdown their freedb/cddb gateway in favour of their own API > > early this year, it likely makes sense to remove support from cdio(1). > > > > CDDB is used to retrieve music CD metadata over the Internet, e.g: > > Artist and Track names. > > > > I left in support for the "cdid" command as it may be useful for > > archival(?) purposes, if not that can go too. > > > > Cc: espie@ as he wrote this code, & maybe he still has music on CDs. > > > > ok? > > > > -Bryan. > > > > [0] http://www.freedb.org/en/ > > [1] > > https://blog.metabrainz.org/2018/09/18/freedb-gateway-end-of-life-notice-march-18-2019/ > > First diff missed a few things.. Not looking for okays now, too soon. Maybe a replacement will show up. Sorry for jumping the gun.
Re: cdio(1): remove CDDB support
On Sat, Dec 28, 2019 at 07:48:47PM -0500, Bryan Steele wrote:
> With FreeDB announcing[0] that the service will be shutting down as of
> March 31st of 2020, and the only other alternative (MusicBrainz) already
> having shutdown their freedb/cddb gateway in favour of their own API
> early this year, it likely makes sense to remove support from cdio(1).
>
> CDDB is used to retrieve music CD metadata over the Internet, e.g:
> Artist and Track names.
>
> I left in support for the "cdid" command as it may be useful for
> archival(?) purposes, if not that can go too.
>
> Cc: espie@ as he wrote this code, & maybe he still has music on CDs.
>
> ok?
>
> -Bryan.
>
> [0] http://www.freedb.org/en/
> [1]
> https://blog.metabrainz.org/2018/09/18/freedb-gateway-end-of-life-notice-march-18-2019/
First diff missed a few things..
Index: Makefile
===
RCS file: /cvs/src/usr.bin/cdio/Makefile,v
retrieving revision 1.6
diff -u -p -u -r1.6 Makefile
--- usr.bin/cdio/Makefile 29 Nov 2008 08:57:10 - 1.6
+++ usr.bin/cdio/Makefile 29 Dec 2019 00:58:58 -
@@ -3,7 +3,7 @@
PROG= cdio
DPADD= ${LIBUTIL} ${LIBEDIT} ${LIBTERMCAP}
LDADD= -lutil -ledit -ltermcap -lsndio
-SRCS= cdio.c cddb.c mmc.c rip.c
+SRCS= cdio.c mmc.c rip.c
CDIAGFLAGS=-Wall -W -Wmissing-prototypes -pedantic
.include
Index: cddb.c
===
RCS file: cddb.c
diff -N cddb.c
--- usr.bin/cdio/cddb.c 7 Dec 2017 02:08:44 - 1.22
+++ /dev/null 1 Jan 1970 00:00:00 -
@@ -1,396 +0,0 @@
-/* $OpenBSD: cddb.c,v 1.22 2017/12/07 02:08:44 krw Exp $ */
-/*
- * Copyright (c) 2002 Marc Espie.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE OPENBSD PROJECT AND CONTRIBUTORS
- * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OPENBSD
- * PROJECT OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include "extern.h"
-
-unsigned long cddb_sum(unsigned long);
-void send_hello(FILE *);
-void send_query(FILE *, int, struct cd_toc_entry *);
-intfurther_query(FILE *, char *);
-intconnect_to(const char *, const char *);
-intparse_connect_to(const char *, const char *);
-char * get_line(FILE *);
-char * get_answer(FILE *);
-void verify_track_names(char **, int, struct cd_toc_entry *);
-void safe_copy(char **, const char *);
-
-unsigned long
-cddb_sum(unsigned long v)
-{
- unsigned long sum = 0;
-
- while (v > 0) {
- sum += v % 10;
- v /= 10;
- }
- return (sum);
-}
-
-unsigned long
-cddb_discid(int n, struct cd_toc_entry *e)
-{
- unsigned long sum;
- int i;
-
- sum = 0;
- for (i =0; i < n; i++)
- sum += cddb_sum(entry2time(e+i));
- return (((sum % 0xff) << 24) |
- ((entry2time(e+n) - entry2time(e)) << 8) | n);
-}
-
-void
-send_hello(FILE *cout)
-{
- char hostname[HOST_NAME_MAX+1];
-
- if (gethostname(hostname, sizeof(hostname)) == -1)
- strlcpy(hostname, "unknown", sizeof hostname);
- fprintf(cout, "CDDB HELLO %s %s cdio " VERSION "\r\n",
- getlogin(), hostname);
- fflush(cout);
-}
-
-void
-send_query(FILE *f, int n, struct cd_toc_entry *e)
-{
- int i;
-
- fprintf(f, "cddb query %8lx %d", cddb_discid(n, e), n);
- for (i = 0; i < n; i++)
- fprintf(f, " %lu", entry2frames(e+i));
- fprintf(f, " %lu\r\n", (entry2frames(e+n)-entry2frames(e)) /75);
- fflus
cdio(1): remove CDDB support
With FreeDB announcing[0] that the service will be shutting down as of
March 31st of 2020, and the only other alternative (MusicBrainz) already
having shutdown their freedb/cddb gateway in favour of their own API
early this year, it likely makes sense to remove support from cdio(1).
CDDB is used to retrieve music CD metadata over the Internet, e.g:
Artist and Track names.
I left in support for the "cdid" command as it may be useful for
archival(?) purposes, if not that can go too.
Cc: espie@ as he wrote this code, & maybe he still has music on CDs.
ok?
-Bryan.
[0] http://www.freedb.org/en/
[1]
https://blog.metabrainz.org/2018/09/18/freedb-gateway-end-of-life-notice-march-18-2019/
Index: Makefile
===
RCS file: /cvs/src/usr.bin/cdio/Makefile,v
retrieving revision 1.6
diff -u -p -u -r1.6 Makefile
--- Makefile29 Nov 2008 08:57:10 - 1.6
+++ Makefile29 Dec 2019 00:34:51 -
@@ -3,7 +3,7 @@
PROG= cdio
DPADD= ${LIBUTIL} ${LIBEDIT} ${LIBTERMCAP}
LDADD= -lutil -ledit -ltermcap -lsndio
-SRCS= cdio.c cddb.c mmc.c rip.c
+SRCS= cdio.c mmc.c rip.c
CDIAGFLAGS=-Wall -W -Wmissing-prototypes -pedantic
.include
Index: cddb.c
===
RCS file: cddb.c
diff -N cddb.c
--- cddb.c 7 Dec 2017 02:08:44 - 1.22
+++ /dev/null 1 Jan 1970 00:00:00 -
@@ -1,396 +0,0 @@
-/* $OpenBSD: cddb.c,v 1.22 2017/12/07 02:08:44 krw Exp $ */
-/*
- * Copyright (c) 2002 Marc Espie.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE OPENBSD PROJECT AND CONTRIBUTORS
- * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OPENBSD
- * PROJECT OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include "extern.h"
-
-unsigned long cddb_sum(unsigned long);
-void send_hello(FILE *);
-void send_query(FILE *, int, struct cd_toc_entry *);
-intfurther_query(FILE *, char *);
-intconnect_to(const char *, const char *);
-intparse_connect_to(const char *, const char *);
-char * get_line(FILE *);
-char * get_answer(FILE *);
-void verify_track_names(char **, int, struct cd_toc_entry *);
-void safe_copy(char **, const char *);
-
-unsigned long
-cddb_sum(unsigned long v)
-{
- unsigned long sum = 0;
-
- while (v > 0) {
- sum += v % 10;
- v /= 10;
- }
- return (sum);
-}
-
-unsigned long
-cddb_discid(int n, struct cd_toc_entry *e)
-{
- unsigned long sum;
- int i;
-
- sum = 0;
- for (i =0; i < n; i++)
- sum += cddb_sum(entry2time(e+i));
- return (((sum % 0xff) << 24) |
- ((entry2time(e+n) - entry2time(e)) << 8) | n);
-}
-
-void
-send_hello(FILE *cout)
-{
- char hostname[HOST_NAME_MAX+1];
-
- if (gethostname(hostname, sizeof(hostname)) == -1)
- strlcpy(hostname, "unknown", sizeof hostname);
- fprintf(cout, "CDDB HELLO %s %s cdio " VERSION "\r\n",
- getlogin(), hostname);
- fflush(cout);
-}
-
-void
-send_query(FILE *f, int n, struct cd_toc_entry *e)
-{
- int i;
-
- fprintf(f, "cddb query %8lx %d", cddb_discid(n, e), n);
- for (i = 0; i < n; i++)
- fprintf(f, " %lu", entry2frames(e+i));
- fprintf(f, " %lu\r\n", (entry2frames(e+n)-entry2frames(e)) /75);
- fflush(f);
-}
-
-#define MAXSIZE 256
-char copy_buffer[MAXSIZE];
-
-void
-safe_copy(char **p, const char *title)
-{
- strnvis(copy_buffer, title, MAXSIZE-1, VIS_TAB|VIS_NL);
- if (*p == NULL)
- *p = strdup(copy_buffer);
- else {
- char *n;
-
- if (asprintf(, "%s%s", *p, copy_buffer) ==
Re: piixpm(4) add support for newer AMD chipsets
On Mon, Dec 16, 2019 at 09:05:47PM +0100, Claudio Jeker wrote:
> On Mon, Dec 16, 2019 at 08:02:55PM +0100, Mark Kettenis wrote:
> > > Date: Mon, 16 Dec 2019 12:37:51 +0100
> > > From: Claudio Jeker
> > >
> > > This diff should add support for newer smbus controllers used on newer AMD
> > > chipsets. Especially Hudson-2 and Kerncz based chipsets. On my Ryzen 5 the
> > > iic(4) busses attach but there is nothing detected on them (well possible
> > > that I missed something). I also implemented the up to 4 busses available
> > > on chipsets of the SBx00 series (on Hudson-2 and Kerncz only 2 ports).
> > >
> > > I would be interested if on systems with Ryzen CPUs something attaches to
> > > those iic(4) busses. Could be that I missed something and fail to properly
> > > access the bus.
> > > --
> > > :wq Claudio
> >
> > Looks good to me. A few nits below. Otherwise ok kettenis@
> >
> > > Index: piixpm.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/piixpm.c,v
> > > retrieving revision 1.39
> > > diff -u -p -r1.39 piixpm.c
> > > --- piixpm.c 1 Oct 2013 20:06:02 - 1.39
> > > +++ piixpm.c 16 Dec 2019 11:26:11 -
> > > @@ -45,15 +45,24 @@
> > > #define PIIXPM_DELAY 200
> > > #define PIIXPM_TIMEOUT 1
> > >
> > > +struct piixpm_smbus {
> > > + int sb_bus;
> > > + struct piixpm_softc *sb_sc;
> > > +};
> > > +
> > > struct piixpm_softc {
> > > struct device sc_dev;
> > >
> > > bus_space_tag_t sc_iot;
> > > bus_space_handle_t sc_ioh;
> > > + bus_space_handle_t sc_sb800_ioh;
> > > void * sc_ih;
> > > int sc_poll;
> > > + int sc_is_sb800;
> > > + int sc_is_kerncz;
> >
> > Can this be called sc_is_hudson2 or maybe sc_is_fch instead? It makes
> > more sense to have this variable describe the oldest variant instead
> > of the newest. I actually think sc_is_fch is the best name.
>
> Changed it to sc_is_fch.
>
> > >
> > > - struct i2c_controller sc_i2c_tag;
> > > + struct piixpm_smbus sc_busses[4];
> > > + struct i2c_controller sc_i2c_tag[4];
> > > struct rwlock sc_i2c_lock;
> > > struct {
> > > i2c_op_t op;
> > > @@ -86,6 +95,7 @@ struct cfdriver piixpm_cd = {
> > >
> > > const struct pci_matchid piixpm_ids[] = {
> > > { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_HUDSON2_SMB },
> > > + { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_KERNCZ_SMB },
> > >
> > > { PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB200_SMB },
> > > { PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB300_SMB },
> > > @@ -117,17 +127,21 @@ piixpm_attach(struct device *parent, str
> > > struct piixpm_softc *sc = (struct piixpm_softc *)self;
> > > struct pci_attach_args *pa = aux;
> > > bus_space_handle_t ioh;
> > > - u_int16_t smb0en;
> > > + u_int16_t val, smb0en;
> > > bus_addr_t base;
> > > pcireg_t conf;
> > > pci_intr_handle_t ih;
> > > const char *intrstr = NULL;
> > > struct i2cbus_attach_args iba;
> > > + int numbusses, i;
> > >
> > > sc->sc_iot = pa->pa_iot;
> > > + numbusses = 1;
> > >
> > > if ((PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
> > > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB) ||
> > > + (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
> > > + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_KERNCZ_SMB) ||
> > > (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_ATI &&
> > > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_ATI_SBX00_SMB &&
> > > PCI_REVISION(pa->pa_class) >= 0x40)) {
> > > @@ -136,10 +150,7 @@ piixpm_attach(struct device *parent, str
> > >* hidden. We need to look at the "SMBus0En" Power
> > >* Management register to find out where they live.
> > >* We use indirect IO access through the index/data
> > > - * pair at 0xcd6/0xcd7 to access "SMBus0En". Since
> > > - * the index/data pair may be needed by other drivers,
> > > - * we only map them for the duration that we actually
> > > - * need them.
> > > + * pair at 0xcd6/0xcd7 to access "SMBus0En".
> > >*/
> >
> > I don't remember why we chose to immediately unmap the registers here.
> > So this may cause some breakage. Probably worth the risk.
>
> If someone wants to write a watchdog driver then something would be needed
> to allow both drivers access to this io range. Nothing else is using the
> piixreg.h file so I doubt this is a big issue. If we hit it than we can
> refactor the drivers.
Yes, I did this because I wasn't sure if some other driver might end
up needing to access those registers. The 0xcd6/0xcd7 pair were only
used to find the SMBus address, as the PCI device had no BARs.
I'm ok with this (and the rest).
> > > if (bus_space_map(sc->sc_iot, SB800_PMREG_BASE,
> > > SB800_PMREG_SIZE, 0, ) != 0) {
> > > @@ -147,29 +158,61 @@ piixpm_attach(struct device
Re: piixpm(4) add support for newer AMD chipsets
On Mon, Dec 16, 2019 at 03:19:30PM +0100, Claudio Jeker wrote: > On Mon, Dec 16, 2019 at 08:46:21AM -0500, Bryan Steele wrote: > > On Mon, Dec 16, 2019 at 12:37:51PM +0100, Claudio Jeker wrote: > > > This diff should add support for newer smbus controllers used on newer AMD > > > chipsets. Especially Hudson-2 and Kerncz based chipsets. On my Ryzen 5 the > > > iic(4) busses attach but there is nothing detected on them (well possible > > > that I missed something). I also implemented the up to 4 busses available > > > on chipsets of the SBx00 series (on Hudson-2 and Kerncz only 2 ports). > > > > > > I would be interested if on systems with Ryzen CPUs something attaches to > > > those iic(4) busses. Could be that I missed something and fail to properly > > > access the bus. > > > -- > > > :wq Claudio > > > > I had a similar diff (except without the additional busses), and didn't > > go further as none of my machines show any devices either (not even > > spdmem(4)), I assumed it was on another bus.. but maybe it's on an > > entirely different controller now. > > > > bios0: vendor American Megatrends Inc. version "5220" date 09/11/2019 > > bios0: ASUSTeK COMPUTER INC. PRIME X470-PRO > > .. > > piixpm0 at pci0 dev 20 function 0 "AMD FCH SMBus" rev 0x59: polling > > iic0 at piixpm0 > > iic1 at piixpm0 > > > > Otherwise, diff "looks" ok. I'll try it on my MateBook later, but > > I have a feeling it was the same story. > > > > Otherwise, diff "looks" ok. > > > > spdmem(4) needs an update to support DDR4 to show up on the bus. I did not > realize that until after I sent this out. This is why spdmem(4) does not > attach. I'm working now on spdmem(4) support for DDR4. Oh interesting. I hadn't considered that. Look forward to testing that then! :-) > -- > :wq Claudio >
Re: piixpm(4) add support for newer AMD chipsets
On Mon, Dec 16, 2019 at 12:37:51PM +0100, Claudio Jeker wrote:
> This diff should add support for newer smbus controllers used on newer AMD
> chipsets. Especially Hudson-2 and Kerncz based chipsets. On my Ryzen 5 the
> iic(4) busses attach but there is nothing detected on them (well possible
> that I missed something). I also implemented the up to 4 busses available
> on chipsets of the SBx00 series (on Hudson-2 and Kerncz only 2 ports).
>
> I would be interested if on systems with Ryzen CPUs something attaches to
> those iic(4) busses. Could be that I missed something and fail to properly
> access the bus.
> --
> :wq Claudio
I had a similar diff (except without the additional busses), and didn't
go further as none of my machines show any devices either (not even
spdmem(4)), I assumed it was on another bus.. but maybe it's on an
entirely different controller now.
bios0: vendor American Megatrends Inc. version "5220" date 09/11/2019
bios0: ASUSTeK COMPUTER INC. PRIME X470-PRO
..
piixpm0 at pci0 dev 20 function 0 "AMD FCH SMBus" rev 0x59: polling
iic0 at piixpm0
iic1 at piixpm0
Otherwise, diff "looks" ok. I'll try it on my MateBook later, but
I have a feeling it was the same story.
Otherwise, diff "looks" ok.
> Index: piixpm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/piixpm.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 piixpm.c
> --- piixpm.c 1 Oct 2013 20:06:02 - 1.39
> +++ piixpm.c 16 Dec 2019 11:26:11 -
> @@ -45,15 +45,24 @@
> #define PIIXPM_DELAY 200
> #define PIIXPM_TIMEOUT 1
>
> +struct piixpm_smbus {
> + int sb_bus;
> + struct piixpm_softc *sb_sc;
> +};
> +
> struct piixpm_softc {
> struct device sc_dev;
>
> bus_space_tag_t sc_iot;
> bus_space_handle_t sc_ioh;
> + bus_space_handle_t sc_sb800_ioh;
> void * sc_ih;
> int sc_poll;
> + int sc_is_sb800;
> + int sc_is_kerncz;
>
> - struct i2c_controller sc_i2c_tag;
> + struct piixpm_smbus sc_busses[4];
> + struct i2c_controller sc_i2c_tag[4];
> struct rwlock sc_i2c_lock;
> struct {
> i2c_op_t op;
> @@ -86,6 +95,7 @@ struct cfdriver piixpm_cd = {
>
> const struct pci_matchid piixpm_ids[] = {
> { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_HUDSON2_SMB },
> + { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_KERNCZ_SMB },
>
> { PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB200_SMB },
> { PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB300_SMB },
> @@ -117,17 +127,21 @@ piixpm_attach(struct device *parent, str
> struct piixpm_softc *sc = (struct piixpm_softc *)self;
> struct pci_attach_args *pa = aux;
> bus_space_handle_t ioh;
> - u_int16_t smb0en;
> + u_int16_t val, smb0en;
> bus_addr_t base;
> pcireg_t conf;
> pci_intr_handle_t ih;
> const char *intrstr = NULL;
> struct i2cbus_attach_args iba;
> + int numbusses, i;
>
> sc->sc_iot = pa->pa_iot;
> + numbusses = 1;
>
> if ((PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
> PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB) ||
> + (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
> + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_KERNCZ_SMB) ||
> (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_ATI &&
> PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_ATI_SBX00_SMB &&
> PCI_REVISION(pa->pa_class) >= 0x40)) {
> @@ -136,10 +150,7 @@ piixpm_attach(struct device *parent, str
>* hidden. We need to look at the "SMBus0En" Power
>* Management register to find out where they live.
>* We use indirect IO access through the index/data
> - * pair at 0xcd6/0xcd7 to access "SMBus0En". Since
> - * the index/data pair may be needed by other drivers,
> - * we only map them for the duration that we actually
> - * need them.
> + * pair at 0xcd6/0xcd7 to access "SMBus0En".
>*/
> if (bus_space_map(sc->sc_iot, SB800_PMREG_BASE,
> SB800_PMREG_SIZE, 0, ) != 0) {
> @@ -147,29 +158,61 @@ piixpm_attach(struct device *parent, str
> return;
> }
>
> - /* Read "SmBus0En" */
> - bus_space_write_1(sc->sc_iot, ioh, 0, SB800_PMREG_SMB0EN);
> - smb0en = bus_space_read_1(sc->sc_iot, ioh, 1);
> - bus_space_write_1(sc->sc_iot, ioh, 0, SB800_PMREG_SMB0EN + 1);
> - smb0en |= (bus_space_read_1(sc->sc_iot, ioh, 1) << 8);
> -
> - bus_space_unmap(sc->sc_iot, ioh, SB800_PMREG_SIZE);
> + if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_AMD &&
> + (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_AMD_HUDSON2_SMB ||
> + PCI_PRODUCT(pa->pa_id) ==
Re: Reduce pledge(2) on file(1)'s main proc
On Fri, Nov 29, 2019 at 11:06:45AM +, Ricardo Mestre wrote:
> Hi,
>
> After fork(2) the main proc needs rpath for {l,}stat/open and sendfd for
> imsg_*
> to send fds to the child proc which is already pledged by recvfd to receive
> them.
>
> Still passes regress tests, OK?
>
> Index: file.c
> ===
> RCS file: /cvs/src/usr.bin/file/file.c,v
> retrieving revision 1.68
> diff -u -p -u -r1.68 file.c
> --- file.c5 Feb 2019 02:17:32 - 1.68
> +++ file.c29 Nov 2019 11:03:47 -
> @@ -207,6 +207,9 @@ main(int argc, char **argv)
> }
> close(pair[1]);
>
> + if (pledge("stdio rpath sendfd", NULL) == -1)
> + err(1, "pledge");
> +
> fclose(magicfp);
> magicfp = NULL;
Nice catch, indeed the parent can reduce pledge(2) here. Works fine as
normal user and with root privdrop.
ok brynet@
Re: OpenBSD 6.x and wxallowed
On Fri, Oct 18, 2019 at 07:39:26AM -0600, Nelson H. F. Beebe wrote: > Because I dislike splitting disks into numerous partitions, each of > whose sizes is a future show-stopper when they prove too small, I > generally split disks into just root + swap. Thus, I find on our > currently 7 versions of OpenBSD 6.x in our test farm reports like > this: > > # mount > /dev/wd0a on / type ffs (local, wxallowed) You are creating your own problem. By default, /usr/local is mounted wxallowed. If you choose to create a single root partition, you're responsible for maintaining your own exploit mitigation countermeasures. > The output of "man mount" says > > wxallowed Processes that ask for memory to be made writeable > plus executable using the mmap(2) and mprotect(2) > system calls are killed by default. This option > allows those processes to continue operation. It is > typically used on the /usr/local filesystem. > > OpenBSD 3.3 introduced the W^X feature in 2004, and some other O/Ses > have implemented it as well since then. > > Has anyone looked into the problem of enumerating packages that are > installed in the /usr/local tree that actually NEED simultaneous write > and execute access? > > If only a small number of packages need W^X capability, would it make > sense to create a separate file tree for them, and let every other > part of the filesystem enjoy W^X protection, along with additional > security from addition of pledge() and veil() promises into software > packages? > > > --- > - Nelson H. F. BeebeTel: +1 801 581 5254 > - > - University of UtahFAX: +1 801 581 4148 > - > - 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: OpenBSD crossed 400,000 commits
On Wed, Oct 09, 2019 at 07:37:11AM +0200, Job Snijders wrote: > On Tue, Oct 08, 2019 at 09:57:42PM -0600, Theo de Raadt wrote: > > Sometime in the last week OpenBSD crossed 400,000 commits (*) upon all > > our repositories since starting at 1995/10/18 08:37:01 > > Canada/Mountain. That's a lot of commits by a lot of amazing people. > > Great achievement! Time to pop a bottle of Opën to celebrate ;-) Personal* pizza party! :] > https://twitter.com/openbsd/status/1181803375479816193/ > > * XL pizza.
New driver for AMD CPU temperature sensor over SMN
Unlike with previous generations of AMD processors, the on-die
temperature sensor is only available from reading from the SMU
co-processor over an internal network (SMN), this includes all
Ryzen CPUs and some later Family 15h models (not supported here).
This adds a new driver based on km(4) and info gleamed from Linux
FreeBSD.
High-TDP Zen/Zen+ parts have undocumented offsets from the control temp
and need to be adjusted, low-TDP (mobile) and Zen2 have no offsets.
I've tested this on a Ryzen 2700X desktop and a Ryzen 5 2500U (mobile)
laptop (no offset). I would appreciate feedback on other models, and
confirmation about Zen2.
Thanks to @nte@bsd.network for the providing helpful information and
much needed prodding.
-Bryan.
Index: arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.478
diff -u -p -u -r1.478 GENERIC
--- sys/arch/amd64/conf/GENERIC 7 Sep 2019 13:46:19 - 1.478
+++ sys/arch/amd64/conf/GENERIC 17 Sep 2019 16:24:03 -
@@ -103,6 +103,7 @@ amdpcib* at pci?# AMD 8111 LPC bridge
tcpcib*at pci? # Intel Atom E600 LPC bridge
kate* at pci? # AMD K8 temperature sensor
km*at pci? # AMD K10 temperature sensor
+kmsmn* at pci? # AMD F17 temperature sensor
amas* at pci? disable # AMD memory configuration
pchtemp* at pci? # Intel C610 termperature sensor
ccp* at pci? # AMD Cryptographic Co-processor
Index: dev/pci/files.pci
===
RCS file: /cvs/src/sys/dev/pci/files.pci,v
retrieving revision 1.338
diff -u -p -u -r1.338 files.pci
--- sys/dev/pci/files.pci 5 Aug 2019 08:33:38 - 1.338
+++ sys/dev/pci/files.pci 17 Sep 2019 16:24:03 -
@@ -774,6 +774,11 @@ device km
attach km at pci
file dev/pci/km.ckm
+# AMD Family 15h/17h Temperature sensor over SMN
+device kmsmn
+attach kmsmn at pci
+file dev/pci/kmsmn.c kmsmn
+
# Intel SOC GCU
device gcu
attach gcu at pci
Index: dev/pci/kmsmn.c
===
RCS file: dev/pci/kmsmn.c
diff -N dev/pci/kmsmn.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ sys/dev/pci/kmsmn.c 17 Sep 2019 16:24:03 -
@@ -0,0 +1,170 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2019 Bryan Steele
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+
+/*
+ * AMD temperature sensors on Family 17h (and some 15h) must be
+ * read from the System Management Unit (SMU) co-processor over
+ * the System Management Network (SMN).
+ */
+
+#define SMN_17H_ADDR_R 0x60
+#define SMN_17H_DATA_R 0x64
+
+/*
+ * AMD Family 17h SMU Thermal Registers (THM)
+ *
+ * 4.2.1, OSRR (Open-Source Register Reference) Guide for Family 17h
+ * [31:21] Current reported temperature.
+ */
+#define SMU_17H_THM0x59800
+#define KM_GET_CURTMP(r) (((r) >> 21) & 0x7ff)
+
+/*
+ * Bit 19 set: "Report on -49C to 206C scale range."
+ * clear: "Report on 0C to 225C (255C?) scale range."
+ */
+#define CURTMP_17H_RANGE_SEL (1 << 19)
+#define CURTMP_17H_RANGE_ADJUST490
+
+/*
+ * Undocumented tCTL offsets gleamed from Linux k10temp driver.
+ */
+struct curtmp_offset {
+ const char *const cpu_model; /* partial match */
+ int tctl_offset;
+} cpu_model_offsets[] = {
+ { "AMD Ryzen 5 1600X", 200 },
+ { "AMD Ryzen 7 1700X", 200 },
+ { "AMD Ryzen 7 1800X", 200 },
+ { "AMD Ryzen 7 2700X", 100 },
+ { "AMD Ryzen Threadripper 19", 270 }, /* many models */
+ { "AMD Ryzen Threadripper 29", 270 }, /* many models */
+ { NULL, 0 },
+ /* ... */
+};
+
+struct kmsmn_softc {
+ struct device sc_dev;
+
+ pci_chipset_tag_t sc_pc;
+ pcitag_tsc_pcitag;
+
+ int sc_tctl_offset;
+ struct mutexsc_smnlock;
+
+ struct ksensor sc_sensor;
+
Re: unveils in ping and traceroute
On Wed, Aug 28, 2019 at 12:03:07PM -0600, Theo de Raadt wrote:
> ping and traceroute are setuid programs, so increased access-reduction
> features are worthwhile.
>
> they can both lock their filesystem visibility to "readonly" very early on.
>
> the attack model being prevented against is very obscure. it imagines a
> bug in something between start-of-program and call-to-pledge (which
> entirely removes filesystem access). implying a getaddrinfo related
> bug. meanwhile, there is privdrop as another protection.
>
> these still feel like improvements.
I think so too. Restricting filesystem access early here only helps.
OK brynet@
> Index: usr.sbin/traceroute/traceroute.c
> ===
> RCS file: /cvs/src/usr.sbin/traceroute/traceroute.c,v
> retrieving revision 1.161
> diff -u -p -u -r1.161 traceroute.c
> --- usr.sbin/traceroute/traceroute.c 28 Jun 2019 13:32:51 - 1.161
> +++ usr.sbin/traceroute/traceroute.c 27 Aug 2019 17:56:56 -
> @@ -327,6 +327,12 @@ main(int argc, char *argv[])
> uid_touid, uid;
> gid_tgid;
>
> + /* Cannot pledge due to special setsockopt()s below */
> + if (unveil("/", "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> +
> if ((conf = calloc(1, sizeof(*conf))) == NULL)
> err(1,NULL);
>
> Index: sbin/ping/ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.237
> diff -u -p -u -r1.237 ping.c
> --- sbin/ping/ping.c 20 Jul 2019 00:49:54 - 1.237
> +++ sbin/ping/ping.c 27 Aug 2019 17:56:17 -
> @@ -264,6 +264,12 @@ main(int argc, char *argv[])
> u_int rtableid = 0;
> extern char *__progname;
>
> + /* Cannot pledge due to special setsockopt()s below */
> + if (unveil("/", "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> +
> if (strcmp("ping6", __progname) == 0) {
> v6flag = 1;
> maxpayload = MAXPAYLOAD6;
>
>
Re: unveil vmd(8)'s priv process
On Mon, Aug 26, 2019 at 11:01:26AM +0100, Ricardo Mestre wrote:
> Hi,
>
> Currently vmd(8) has 3 processes that run under chroot(2)/chdir(2), namely
> control, vmm and priv. From these both control and vmm already run under
> different pledge(2)s but without any filesystem access, priv in the other hand
> cannot use pledge due to forbidden ioctls.
>
> That being said the priv proc can instead just run with an unveil("/", "") to
> disable fs access, and while here remove the need of chroot/chdir dance since
> it's not needed any longer.
>
> I've been running this for more than a week now without issues. Any
> comments/objections? OK?
>
> /mestre
Tested here, no problems with my local VMs. Confirm that the priv
process is now unveiled.
root 12241 0.0 0.0 1224 1780 ?? SU 9:11AM0:00.01 vmd:
priv (vmd)
Just to clarify, this diff is removing chroot(2) from 3 processes:
* priv - (cannot be pledged)
* control - pledged "stdio,unix,sendfd,recvfd"
* vmm - pledged "stdio,proc,sendfd,recvfd,vmm"
I think this should be OK.
-Bryan.
> Index: priv.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/priv.c,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 priv.c
> --- priv.c28 Jun 2019 13:32:51 - 1.15
> +++ priv.c19 Aug 2019 09:38:51 -
> @@ -66,8 +66,14 @@ priv_run(struct privsep *ps, struct priv
>
> /*
>* no pledge(2) in the "priv" process:
> - * write ioctls are not permitted by pledge.
> + * write ioctls are not permitted by pledge, but we can restrict fs
> + * access with unveil
>*/
> + /* no filesystem visibility */
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
>
> /* Open our own socket for generic interface ioctls */
> if ((env->vmd_fd = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
> Index: proc.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/proc.c,v
> retrieving revision 1.18
> diff -u -p -u -r1.18 proc.c
> --- proc.c10 Sep 2018 10:36:01 - 1.18
> +++ proc.c19 Aug 2019 09:38:51 -
> @@ -524,7 +524,6 @@ proc_run(struct privsep *ps, struct priv
> void (*run)(struct privsep *, struct privsep_proc *, void *), void *arg)
> {
> struct passwd *pw;
> - const char *root;
> struct control_sock *rcs;
>
> log_procinit(p->p_title);
> @@ -545,17 +544,6 @@ proc_run(struct privsep *ps, struct priv
> pw = p->p_pw;
> else
> pw = ps->ps_pw;
> -
> - /* Change root directory */
> - if (p->p_chroot != NULL)
> - root = p->p_chroot;
> - else
> - root = pw->pw_dir;
> -
> - if (chroot(root) == -1)
> - fatal("%s: chroot", __func__);
> - if (chdir("/") == -1)
> - fatal("%s: chdir(\"/\")", __func__);
>
> privsep_process = p->p_id;
>
> Index: proc.h
> ===
> RCS file: /cvs/src/usr.sbin/vmd/proc.h,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 proc.h
> --- proc.h10 Sep 2018 10:36:01 - 1.16
> +++ proc.h19 Aug 2019 09:38:51 -
> @@ -136,7 +136,6 @@ struct privsep_proc {
> void(*p_init)(struct privsep *,
> struct privsep_proc *);
> void(*p_shutdown)(void);
> - const char *p_chroot;
> struct passwd *p_pw;
> struct privsep *p_ps;
> };
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.115
> diff -u -p -u -r1.115 vmd.c
> --- vmd.c 14 Aug 2019 07:34:49 - 1.115
> +++ vmd.c 19 Aug 2019 09:38:51 -
> @@ -789,9 +789,8 @@ main(int argc, char **argv)
> if ((ps->ps_pw = getpwnam(VMD_USER)) == NULL)
> fatal("unknown user %s", VMD_USER);
>
> - /* First proc runs as root without pledge but in default chroot */
> + /* First proc runs as root without pledge but with unveil */
> proc_priv->p_pw = _privpw; /* initialized to all 0 */
> - proc_priv->p_chroot = ps->ps_pw->pw_dir; /* from VMD_USER */
>
> /* Open /dev/vmm */
> if (env->vmd_noaction == 0) {
> Index: vmm.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.93
> diff -u -p -u -r1.93 vmm.c
> --- vmm.c 28 Jun 2019 13:32:51 - 1.93
> +++ vmm.c 19 Aug 2019 09:38:51 -
> @@ -655,7 +655,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t
> if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, fds) == -1)
> fatal("socketpair");
>
> - /* Start child vmd for this VM (fork, chroot, drop privs) */
> + /* Start child
Re: TSC synchronization on MP machines
On Mon, Aug 05, 2019 at 04:58:27PM +0300, Paul Irofti wrote:
> Hi,
>
> Here is a third version of the TSC diff that also take into
> consideration the suspend-resume path which was ignored by the previous
> thus rendering resume broken.
>
> Have a go at it. Reports are welcome. So far I only got ONE report from
> a machine with broken TSC :(
>
> Paul
>
>
> Index: arch/amd64/amd64/acpi_machdep.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/acpi_machdep.c,v
> retrieving revision 1.86
> diff -u -p -u -p -r1.86 acpi_machdep.c
> --- arch/amd64/amd64/acpi_machdep.c 23 Oct 2018 17:51:32 - 1.86
> +++ arch/amd64/amd64/acpi_machdep.c 5 Aug 2019 13:54:33 -
> @@ -60,6 +60,8 @@ extern paddr_t tramp_pdirpa;
>
> extern int acpi_savecpu(void) __returns_twice;
>
> +extern int64_t tsc_drift_observed;
> +
> #define ACPI_BIOS_RSDP_WINDOW_BASE0xe
> #define ACPI_BIOS_RSDP_WINDOW_SIZE0x2
>
> @@ -481,6 +483,8 @@ acpi_resume_cpu(struct acpi_softc *sc)
> {
> fpuinit(_info_primary);
>
> + cpu_info_primary.cpu_cc_skew = 0; /* futile */
> + tsc_drift_observed = 0; /* reset tsc drift on resume */
> cpu_init(_info_primary);
> cpu_ucode_apply(_info_primary);
>
> Index: arch/amd64/amd64/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.137
> diff -u -p -u -p -r1.137 cpu.c
> --- arch/amd64/amd64/cpu.c28 May 2019 18:17:01 - 1.137
> +++ arch/amd64/amd64/cpu.c5 Aug 2019 13:54:34 -
> @@ -754,6 +754,10 @@ cpu_init(struct cpu_info *ci)
> cr4 = rcr4();
> lcr4(cr4 & ~CR4_PGE);
> lcr4(cr4);
> +
> + /* Synchronize TSC */
> + if (cold && !CPU_IS_PRIMARY(ci))
> + tsc_sync_ap(ci);
> #endif
> }
>
> @@ -808,6 +812,7 @@ void
> cpu_start_secondary(struct cpu_info *ci)
> {
> int i;
> + u_long s;
>
> ci->ci_flags |= CPUF_AP;
>
> @@ -828,6 +833,17 @@ cpu_start_secondary(struct cpu_info *ci)
> printf("dropping into debugger; continue from here to resume
> boot\n");
> db_enter();
> #endif
> + } else {
> + /*
> + * Synchronize time stamp counters. Invalidate cache and do
> + * twice (in tsc_sync_bp) to minimize possible cache effects.
> + * Disable interrupts to try and rule out any external
> + * interference.
> + */
> + s = intr_disable();
> + wbinvd();
> + tsc_sync_bp(ci);
> + intr_restore(s);
> }
>
> if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
> @@ -852,6 +868,8 @@ void
> cpu_boot_secondary(struct cpu_info *ci)
> {
> int i;
> + int64_t drift;
> + u_long s;
>
> atomic_setbits_int(>ci_flags, CPUF_GO);
>
> @@ -864,6 +882,17 @@ cpu_boot_secondary(struct cpu_info *ci)
> printf("dropping into debugger; continue from here to resume
> boot\n");
> db_enter();
> #endif
> + } else if (cold) {
> + /* Synchronize TSC again, check for drift. */
> + drift = ci->cpu_cc_skew;
> + s = intr_disable();
> + wbinvd();
> + tsc_sync_bp(ci);
> + intr_restore(s);
> + drift -= ci->cpu_cc_skew;
> + printf("TSC skew=%lld drift=%lld\n",
> + (long long)ci->cpu_cc_skew, (long long)drift);
> + tsc_sync_drift(drift);
> }
> }
>
> @@ -888,7 +917,14 @@ cpu_hatch(void *v)
> panic("%s: already running!?", ci->ci_dev->dv_xname);
> #endif
>
> + /*
> + * Synchronize the TSC for the first time. Note that interrupts are
> + * off at this point.
> + */
> + wbinvd();
> ci->ci_flags |= CPUF_PRESENT;
> + ci->cpu_cc_skew = 0;/* reset on resume */
> + tsc_sync_ap(ci);
>
> lapic_enable();
> lapic_startclock();
> Index: arch/amd64/amd64/tsc.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 tsc.c
> --- arch/amd64/amd64/tsc.c6 Jun 2019 19:43:35 - 1.11
> +++ arch/amd64/amd64/tsc.c5 Aug 2019 13:54:34 -
> @@ -1,8 +1,10 @@
> /* $OpenBSD: tsc.c,v 1.11 2019/06/06 19:43:35 kettenis Exp $ */
> /*
> + * Copyright (c) 2008 The NetBSD Foundation, Inc.
> * Copyright (c) 2016,2017 Reyk Floeter
> * Copyright (c) 2017 Adam Steen
> * Copyright (c) 2017 Mike Belopuhov
> + * Copyright (c) 2019 Paul Irofti
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -20,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -33,6 +36,13 @@
Re: TSC synchronization on MP machines
On Fri, Aug 02, 2019 at 01:29:37PM +0300, Paul Irofti wrote:
> Hi,
>
> Here is an updated diff with a few bugs eliminated from the previous and
> with most of the concerns I got in private and from Mark fixed.
>
> I will do the TSC_ADJUST_MSR dance in another iteration if the current
> incarnation turns out to be correct for machines suffering from TSCs not
> in sync.
>
> The thing I am mostly worried about now is in the following sum
>
> uint
> tsc_get_timecount(struct timecounter *tc)
> {
> return rdtsc() + curcpu()->cpu_cc_skew;
> }
>
> can one term be executed on one CPU and the other on another? Is there a
> way to protect this from happening other than locking?
>
> I see NetBSD is checking for a change in the number of context switches
> of the current process.
>
> My plan is to have a fix in the tree before 6.6 is released, so I would
> love to hear your thoughts and reports on this.
>
> Thanks,
> Paul
>
>
> Index: arch/amd64/amd64/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.137
> diff -u -p -u -p -r1.137 cpu.c
> --- arch/amd64/amd64/cpu.c28 May 2019 18:17:01 - 1.137
> +++ arch/amd64/amd64/cpu.c2 Aug 2019 10:25:04 -
> @@ -754,6 +754,10 @@ cpu_init(struct cpu_info *ci)
> cr4 = rcr4();
> lcr4(cr4 & ~CR4_PGE);
> lcr4(cr4);
> +
> + /* Synchronize TSC */
> + if (!CPU_IS_PRIMARY(ci))
> + tsc_sync_ap(ci);
> #endif
> }
>
> @@ -808,6 +812,7 @@ void
> cpu_start_secondary(struct cpu_info *ci)
> {
> int i;
> + u_long s;
>
> ci->ci_flags |= CPUF_AP;
>
> @@ -828,6 +833,17 @@ cpu_start_secondary(struct cpu_info *ci)
> printf("dropping into debugger; continue from here to resume
> boot\n");
> db_enter();
> #endif
> + } else {
> + /*
> + * Synchronize time stamp counters. Invalidate cache and do
> + * twice (in tsc_sync_bp) to minimize possible cache effects.
> + * Disable interrupts to try and rule out any external
> + * interference.
> + */
> + s = intr_disable();
> + wbinvd();
> + tsc_sync_bp(ci);
> + intr_restore(s);
> }
>
> if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
> @@ -852,6 +868,8 @@ void
> cpu_boot_secondary(struct cpu_info *ci)
> {
> int i;
> + int64_t drift;
> + u_long s;
>
> atomic_setbits_int(>ci_flags, CPUF_GO);
>
> @@ -864,6 +882,17 @@ cpu_boot_secondary(struct cpu_info *ci)
> printf("dropping into debugger; continue from here to resume
> boot\n");
> db_enter();
> #endif
> + } else {
> + /* Synchronize TSC again, check for drift. */
> + drift = ci->cpu_cc_skew;
> + s = intr_disable();
> + wbinvd();
> + tsc_sync_bp(ci);
> + intr_restore(s);
> + drift -= ci->cpu_cc_skew;
> + printf("TSC skew=%lld drift=%lld\n",
> + (long long)ci->cpu_cc_skew, (long long)drift);
> + tsc_sync_drift(drift);
> }
> }
>
> @@ -888,7 +917,13 @@ cpu_hatch(void *v)
> panic("%s: already running!?", ci->ci_dev->dv_xname);
> #endif
>
> + /*
> + * Synchronize the TSC for the first time. Note that interrupts are
> + * off at this point.
> + */
> + wbinvd();
> ci->ci_flags |= CPUF_PRESENT;
> + tsc_sync_ap(ci);
>
> lapic_enable();
> lapic_startclock();
> Index: arch/amd64/amd64/tsc.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 tsc.c
> --- arch/amd64/amd64/tsc.c6 Jun 2019 19:43:35 - 1.11
> +++ arch/amd64/amd64/tsc.c2 Aug 2019 10:25:04 -
> @@ -1,8 +1,10 @@
> /* $OpenBSD: tsc.c,v 1.11 2019/06/06 19:43:35 kettenis Exp $ */
> /*
> + * Copyright (c) 2008 The NetBSD Foundation, Inc.
> * Copyright (c) 2016,2017 Reyk Floeter
> * Copyright (c) 2017 Adam Steen
> * Copyright (c) 2017 Mike Belopuhov
> + * Copyright (c) 2019 Paul Irofti
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -20,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -33,6 +36,13 @@ inttsc_recalibrate;
> uint64_t tsc_frequency;
> int tsc_is_invariant;
>
> +int64_t tsc_drift_max = 250;/* max cycles */
> +int64_t tsc_drift_observed;
> +bool tsc_good;
> +
> +volatile int64_t tsc_sync_val;
> +volatile struct cpu_info *tsc_sync_cpu;
> +
> uint tsc_get_timecount(struct timecounter *tc);
>
> struct timecounter tsc_timecounter = {
> @@ -172,10 +182,8 @@
Re: unveil in process accounting and lastcomm
On Thu, Jul 25, 2019 at 10:06:52AM -0400, Bryan Steele wrote:
> On Thu, Jul 18, 2019 at 05:44:21PM -0400, Bryan Steele wrote:
> > On Thu, Jul 18, 2019 at 11:46:46AM -0400, Bryan Steele wrote:
> > > On Thu, Jul 18, 2019 at 04:13:10PM +0200, Alexander Bluhm wrote:
> > > > Hi,
> > > >
> > > > Can we track unveil(2) violators in process accounting lastcomm(1)?
> > > > This makes it easier to find them.
> > > >
> > > > $ lastcomm | grep -e '-[A-Z]U'
> > > > pflogd -FU root__ 0.00 secs Thu Jul 18 14:19
> > > > (2:33:22.00)
> > > >
> > > > Seems that pflogd(8) has to be investigated.
> > >
> > > Interesting.
> > >
> > > This appears to be a false positive, libpcap will first attempt to open
> > > /dev/bpf as O_RDWR and then falls back to O_RDONLY on EACCES. I'm fairly
> > > confident that pflogd(8) does not need write access to bpf. If anything,
> > > unveil(2) appears to have found an old bug here, as before pflogd was
> > > always opening the device with both read/write permissions.
> > >
> > > tcpdump avoids this by internalizing more parts of libpcap, and also
> > > opening /dev/bpf O_RDONLY itself.
> > >
> > > spamlogd appears to be the only other user of pcap_open_live() in base,
> > > unfortunately it calls unveil after, so /dev/bpf is opened O_RDWR. I
> > > don't use spamlogd, though.
> >
> > Here's a potential diff for both pflogd and spamlogd, I've tested it
> > works with pflogd. I only compile tested spamlogd. But it should avoid
> > the unveil accounting errors.
> >
> > This duplicates a lot of code into both, but I don't feel like trying
> > to extent the libpcap API for this.
> >
> > comments? ok?
>
> With bluhm@'s unveil accounting diff, is anyone ok wit this this
> approach to fixing the issue? These are the only pcap_open_live(3)
> consumers in base, but I don't feel comfortable trying to extend
> the libpcap API, and pretty much everything is already poking at
> libpcap internals due to bad API design for privsep.
>
> -Bryan.
Here's a new diff that renames the local function to avoid conflicting
with future attempts at improving the libpcap API. Requested by deraadt@
-Bryan.
Index: pflogd.c
===
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.59
diff -u -p -u -r1.59 pflogd.c
--- sbin/pflogd/pflogd.c26 Aug 2018 18:24:46 - 1.59
+++ sbin/pflogd/pflogd.c25 Jul 2019 14:23:14 -
@@ -73,6 +73,7 @@ void dump_packet(u_char *, const struct
void dump_packet_nobuf(u_char *, const struct pcap_pkthdr *, const u_char *);
int flush_buffer(FILE *);
int if_exists(char *);
+pcap_t *pflog_read_live(const char *, int, int, int, char *);
void logmsg(int, const char *, ...);
void purge_buffer(void);
int reset_dump(void);
@@ -194,10 +195,95 @@ if_exists(char *ifname)
return (if_nametoindex(ifname) != 0);
}
+pcap_t *
+pflog_read_live(const char *source, int slen, int promisc, int to_ms,
+char *ebuf)
+{
+ int fd;
+ struct bpf_version bv;
+ struct ifreqifr;
+ u_int v, dlt = DLT_PFLOG;
+ pcap_t *p;
+
+ if (source == NULL || slen <= 0)
+ return (NULL);
+
+ p = pcap_create(source, ebuf);
+ if (p == NULL)
+ return (NULL);
+
+ /* Open bpf(4) read only */
+ if ((fd = open("/dev/bpf", O_RDONLY)) == -1)
+ return (NULL);
+
+ if (ioctl(fd, BIOCVERSION, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCVERSION: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (bv.bv_major != BPF_MAJOR_VERSION ||
+ bv.bv_minor < BPF_MINOR_VERSION) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE,
+ "kernel bpf filter out of date");
+ goto bad;
+ }
+
+ strlcpy(ifr.ifr_name, source, sizeof(ifr.ifr_name));
+ if (ioctl(fd, BIOCSETIF, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSETIF: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (dlt != (u_int) -1 && ioctl(fd, BIOCSDLT, )) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSDLT: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ p->fd = fd;
+ p->snapshot = slen;
+ p->linktype = dlt;
+
+ /* set timeout */
+ if (to_ms != 0) {
+ struct timeval to;
+ to.tv_sec = to_ms / 1000;
+
Re: unveil in process accounting and lastcomm
On Thu, Jul 18, 2019 at 05:44:21PM -0400, Bryan Steele wrote:
> On Thu, Jul 18, 2019 at 11:46:46AM -0400, Bryan Steele wrote:
> > On Thu, Jul 18, 2019 at 04:13:10PM +0200, Alexander Bluhm wrote:
> > > Hi,
> > >
> > > Can we track unveil(2) violators in process accounting lastcomm(1)?
> > > This makes it easier to find them.
> > >
> > > $ lastcomm | grep -e '-[A-Z]U'
> > > pflogd -FU root__ 0.00 secs Thu Jul 18 14:19
> > > (2:33:22.00)
> > >
> > > Seems that pflogd(8) has to be investigated.
> >
> > Interesting.
> >
> > This appears to be a false positive, libpcap will first attempt to open
> > /dev/bpf as O_RDWR and then falls back to O_RDONLY on EACCES. I'm fairly
> > confident that pflogd(8) does not need write access to bpf. If anything,
> > unveil(2) appears to have found an old bug here, as before pflogd was
> > always opening the device with both read/write permissions.
> >
> > tcpdump avoids this by internalizing more parts of libpcap, and also
> > opening /dev/bpf O_RDONLY itself.
> >
> > spamlogd appears to be the only other user of pcap_open_live() in base,
> > unfortunately it calls unveil after, so /dev/bpf is opened O_RDWR. I
> > don't use spamlogd, though.
>
> Here's a potential diff for both pflogd and spamlogd, I've tested it
> works with pflogd. I only compile tested spamlogd. But it should avoid
> the unveil accounting errors.
>
> This duplicates a lot of code into both, but I don't feel like trying
> to extent the libpcap API for this.
>
> comments? ok?
With bluhm@'s unveil accounting diff, is anyone ok wit this this
approach to fixing the issue? These are the only pcap_open_live(3)
consumers in base, but I don't feel comfortable trying to extend
the libpcap API, and pretty much everything is already poking at
libpcap internals due to bad API design for privsep.
I'd appreciate if someone could test spamlogd(8) still works.
-Bryan.
Index: pflogd.c
===
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.59
diff -u -p -r1.59 pflogd.c
--- sbin/pflogd/pflogd.c26 Aug 2018 18:24:46 - 1.59
+++ sbin/pflogd/pflogd.c25 Jul 2019 14:00:02 -
@@ -73,6 +73,7 @@ void dump_packet(u_char *, const struct
void dump_packet_nobuf(u_char *, const struct pcap_pkthdr *, const u_char *);
int flush_buffer(FILE *);
int if_exists(char *);
+pcap_t *pcap_live(const char *, int, int, int, char *);
void logmsg(int, const char *, ...);
void purge_buffer(void);
int reset_dump(void);
@@ -194,10 +195,95 @@ if_exists(char *ifname)
return (if_nametoindex(ifname) != 0);
}
+pcap_t *
+pcap_live(const char *source, int slen, int promisc, int to_ms,
+char *ebuf)
+{
+ int fd;
+ struct bpf_version bv;
+ struct ifreqifr;
+ u_int v, dlt = DLT_PFLOG;
+ pcap_t *p;
+
+ if (source == NULL || slen <= 0)
+ return (NULL);
+
+ p = pcap_create(source, ebuf);
+ if (p == NULL)
+ return (NULL);
+
+ /* Open bpf(4) read only */
+ if ((fd = open("/dev/bpf", O_RDONLY)) == -1)
+ return (NULL);
+
+ if (ioctl(fd, BIOCVERSION, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCVERSION: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (bv.bv_major != BPF_MAJOR_VERSION ||
+ bv.bv_minor < BPF_MINOR_VERSION) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE,
+ "kernel bpf filter out of date");
+ goto bad;
+ }
+
+ strlcpy(ifr.ifr_name, source, sizeof(ifr.ifr_name));
+ if (ioctl(fd, BIOCSETIF, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSETIF: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (dlt != (u_int) -1 && ioctl(fd, BIOCSDLT, )) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSDLT: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ p->fd = fd;
+ p->snapshot = slen;
+ p->linktype = dlt;
+
+ /* set timeout */
+ if (to_ms != 0) {
+ struct timeval to;
+ to.tv_sec = to_ms / 1000;
+ to.tv_usec = (to_ms * 1000) % 100;
+ if (ioctl(p->fd, BIOCSRTIMEOUT, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSRTIMEOUT: %s",
+ pcap_strerror(errno));
+ }
+ }
+ if (promisc)
+ /* this is allowed to fail */
+ ioctl(fd, BI
Re: [patch] Re-add 'proc' to vi(1) when running in secure mode.
On Sun, Jul 21, 2019 at 05:57:32PM +0200, Ingo Schwarze wrote: > Hi, > > Bryan Steele wrote on Fri, Jul 19, 2019 at 06:14:56PM -0400: > > On Sat, Jul 20, 2019 at 12:03:03AM +0200, Jesper Wallin wrote: > >> On Fri, Jul 19, 2019 at 05:14:03PM -0400, Bryan Steele wrote: > > >>> I suspect that in secure/-S mode, the :pre[serve] should either be > >>> disabled, or modified to stop calling sendmail. The mail it is sending > >>> is purely advisory, and should be easy to disable. See common/recover.c. > > >> Oh, you're right. A bit ironic that I didn't notice the exec violation > >> due to the fork being permitted now. Thanks for pointing this out! > >> Scrap my old patch, here's a better proposal: > > > ok brynet@ > > No, that patch is not OK either. > > It breaks :pre in -S mode because rcv_mailfile() is a blantant misnomer > that actually has two purposes: > 1. Create the mail file to indicate that there is something recoverable > 2. and then actually send the mail. > > While step 2 must be skipped, step 1 for creating the recover.* file > is still needed, or "vi -r" will later complain "vi: No files to recover" > even though a vi.* file exists in /tmp/vi.recover/. Yes, i freely > admit that the design is horribly contorted. > > So here is a better patch avoiding that problem. > It is also safer because it is easier to see that fork(2) > can no longer be reached. Otherwise, you would need to understand > that even though rcv_init() calls rcv_mailfile() and rcv_mailfile() > calls rcv_email(), fork(2) cannot be reached in that way because > isync is 0 in rcv_mailfile(). > > Yours, > Ingo > > P.S. > See below the patch for my analysis of the code, which you may or may > not find helpful while verifying the patch. Nice catch and analysis Ingo, I somehow missed that. Indeed, moving the check into rcv_email before fork makes more sense. Sorry for jumping the gun. -Bryan. > Index: common/recover.c > === > RCS file: /cvs/src/usr.bin/vi/common/recover.c,v > retrieving revision 1.29 > diff -u -p -r1.29 recover.c > --- common/recover.c 10 Nov 2017 18:25:48 - 1.29 > +++ common/recover.c 21 Jul 2019 15:45:59 - > @@ -821,6 +821,15 @@ rcv_email(SCR *sp, int fd) > struct stat sb; > pid_t pid; > > + /* > + * In secure mode, our pledge(2) includes neither "proc" > + * nor "exec". So simply skip sending the mail. > + * Later vi -r still works because rcv_mailfile() > + * already did all the necessary setup. > + */ > + if (O_ISSET(sp, O_SECURE)) > + return; > + > if (_PATH_SENDMAIL[0] != '/' || stat(_PATH_SENDMAIL, ) == -1) > msgq_str(sp, M_SYSERR, > _PATH_SENDMAIL, "not sending email: %s"); > > > > There are three call paths to rcv_mailfile() and rcv_email(): > > * rcv_init() -> rcv_mailfile() >purpose: set up .rcv_fd and .rcv_mpath > for locking purposes and in case of later dying from a signal >calls to rcv_init() are always safe because isync == 0, > i.e. rcv_email() is never called > > * rcv_sync() -> rcv_email() directly >purpose: emergency saving while dying from SIGTERM; > RCV_EMAIL is not set in any other situation >so the direct call to rcv_email() must be neutered >note: RCV_SNAPSHOT is NOT set in this case > > * rcv_sync() -> rcv_mailfile() -> rcv_email() >purpose: manual "pre" command > RCV_SNAPSHOT is not set in any other situation >note: RCV_EMAIL is NOT set in this case >here, the rcv_email() inside rcv_mailfile() must be neutered > >
Re: [patch] Re-add 'proc' to vi(1) when running in secure mode.
On Sat, Jul 20, 2019 at 12:03:03AM +0200, Jesper Wallin wrote:
> On Fri, Jul 19, 2019 at 05:14:03PM -0400, Bryan Steele wrote:
> > I suspect that in secure/-S mode, the :pre[serve] should either be
> > disabled, or modified to stop calling sendmail. The mail it is sending
> > is purely advisory, and should be easy to disable. See common/recover.c.
>
> Oh, you're right. A bit ironic that I didn't notice the exec violation
> due to the fork being permitted now. Thanks for pointing this out!
>
> Scrap my old patch, here's a better proposal:
>
>
> Index: common/recover.c
> ===
> RCS file: /cvs/src/usr.bin/vi/common/recover.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 recover.c
> --- common/recover.c 10 Nov 2017 18:25:48 - 1.29
> +++ common/recover.c 19 Jul 2019 21:57:16 -
> @@ -264,7 +264,7 @@ rcv_sync(SCR *sp, u_int flags)
> F_SET(ep, F_RCV_NORM);
>
> /* REQUEST: send email. */
> - if (LF_ISSET(RCV_EMAIL))
> + if (O_ISSET(sp, O_SECURE) == 0 && LF_ISSET(RCV_EMAIL))
> rcv_email(sp, ep->rcv_fd);
> }
>
> @@ -289,7 +289,8 @@ rcv_sync(SCR *sp, u_int flags)
> sp->gp->scr_busy(sp,
> "Copying file for recovery...", BUSY_ON);
> if (rcv_copy(sp, fd, ep->rcv_path) ||
> - close(fd) || rcv_mailfile(sp, 1, buf)) {
> + close(fd) || (O_ISSET(sp, O_SECURE) == 0 &&
> + rcv_mailfile(sp, 1, buf))) {
> (void)unlink(buf);
> (void)close(fd);
> rval = 1;
ok brynet@
Re: [patch] Re-add 'proc' to vi(1) when running in secure mode.
On Fri, Jul 19, 2019 at 09:43:14PM +0200, Jesper Wallin wrote:
> Hi all,
>
> When using vi(1) with secure mode (-S), both 'proc' and 'exec' are
> stripped from the pledge promise. This breaks the :pre[serve] command
> as it uses fork(2). This is broken on 6.4, 6.5 and -current.
>
> Re-add the 'proc' promise, even when running in secure mode.
>
>
> Jesper Wallin
vi(1) is calling fork(2) here because it intends to exec the sendmail
wrapper, which will not succeed without the exec promise.
50282 vi CALL stat(0xb0a2508fb5,0x7f7e3e80)
50282 vi NAMI "/usr/sbin/sendmail"
50282 vi STRU struct stat { dev=1029, ino=103994,
mode=-r-xr-xr-x , nlin
k=1, uid=0<"root">, gid=7<"bin">, rdev=419648, atime=1562946228<"Jul 12
11:43:48
2019">, mtime=1562946228<"Jul 12 11:43:48 2019">, ctime=1562956860<"Jul
12 14:4
1:00 2019">.345836594, size=10696, blocks=24, blksize=16384, flags=0x0,
gen=0x0
}
50282 vi RET stat 0
50282 vi CALL kbind(0x7f7e3db0,24,0xcfec3cf125b97ff7)
50282 vi RET kbind 0
50282 vi CALL fork()
50282 vi PLDG fork, "proc", errno 1 Operation not permitted
50282 vi PSIG SIGABRT SIG_DFL code <1210892288>
50282 vi NAMI "vi.core"
In the non-secure case, you'll see:
78700 vi CALL execve(0xe73ebd08fb5,0x7f7b9340,0xe76e34b8300)
78700 vi NAMI "/usr/sbin/sendmail"
78700 vi ARGS
[0] = "sendmail"
[1] = "-t"
..
I suspect that in secure/-S mode, the :pre[serve] should either be
disabled, or modified to stop calling sendmail. The mail it is sending
is purely advisory, and should be easy to disable. See common/recover.c.
-Bryan.
Re: unveil in process accounting and lastcomm
On Thu, Jul 18, 2019 at 11:46:46AM -0400, Bryan Steele wrote:
> On Thu, Jul 18, 2019 at 04:13:10PM +0200, Alexander Bluhm wrote:
> > Hi,
> >
> > Can we track unveil(2) violators in process accounting lastcomm(1)?
> > This makes it easier to find them.
> >
> > $ lastcomm | grep -e '-[A-Z]U'
> > pflogd -FU root__ 0.00 secs Thu Jul 18 14:19
> > (2:33:22.00)
> >
> > Seems that pflogd(8) has to be investigated.
>
> Interesting.
>
> This appears to be a false positive, libpcap will first attempt to open
> /dev/bpf as O_RDWR and then falls back to O_RDONLY on EACCES. I'm fairly
> confident that pflogd(8) does not need write access to bpf. If anything,
> unveil(2) appears to have found an old bug here, as before pflogd was
> always opening the device with both read/write permissions.
>
> tcpdump avoids this by internalizing more parts of libpcap, and also
> opening /dev/bpf O_RDONLY itself.
>
> spamlogd appears to be the only other user of pcap_open_live() in base,
> unfortunately it calls unveil after, so /dev/bpf is opened O_RDWR. I
> don't use spamlogd, though.
Here's a potential diff for both pflogd and spamlogd, I've tested it
works with pflogd. I only compile tested spamlogd. But it should avoid
the unveil accounting errors.
This duplicates a lot of code into both, but I don't feel like trying
to extent the libpcap API for this.
comments? ok?
Index: pflogd.c
===
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.59
diff -u -p -u -r1.59 pflogd.c
--- sbin/pflogd/pflogd.c26 Aug 2018 18:24:46 - 1.59
+++ sbin/pflogd/pflogd.c18 Jul 2019 21:30:39 -
@@ -73,6 +73,7 @@ void dump_packet(u_char *, const struct
void dump_packet_nobuf(u_char *, const struct pcap_pkthdr *, const u_char *);
int flush_buffer(FILE *);
int if_exists(char *);
+pcap_t *pcap_live(const char *, int, int, int, char *);
void logmsg(int, const char *, ...);
void purge_buffer(void);
int reset_dump(void);
@@ -194,10 +195,97 @@ if_exists(char *ifname)
return (if_nametoindex(ifname) != 0);
}
+pcap_t *
+pcap_live(const char *source, int slen, int promisc, int to_ms,
+char *ebuf)
+{
+ int fd;
+ struct bpf_version bv;
+ struct ifreqifr;
+ u_int v, dlt = DLT_PFLOG;
+ pcap_t *p;
+
+ if (source == NULL || slen <= 0)
+ return (NULL);
+
+ p = pcap_create(source, ebuf);
+ if (p == NULL)
+ return (NULL);
+
+ /* Open bpf(4) read only */
+ if ((fd = open("/dev/bpf", O_RDONLY)) == -1)
+ return (NULL);
+
+ if (ioctl(fd, BIOCVERSION, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCVERSION: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (bv.bv_major != BPF_MAJOR_VERSION ||
+ bv.bv_minor < BPF_MINOR_VERSION) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE,
+ "kernel bpf filter out of date");
+ goto bad;
+ }
+
+ strlcpy(ifr.ifr_name, source, sizeof(ifr.ifr_name));
+ if (ioctl(fd, BIOCSETIF, ) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSETIF: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (dlt != (u_int) -1 && ioctl(fd, BIOCSDLT, )) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSDLT: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ p->fd = fd;
+ p->snapshot = slen;
+ p->linktype = dlt;
+
+ /* set timeout */
+ if (to_ms != 0) {
+ struct timeval to;
+ to.tv_sec = to_ms / 1000;
+ to.tv_usec = (to_ms * 1000) % 100;
+ if (ioctl(p->fd, BIOCSRTIMEOUT, ) < 0) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSRTIMEOUT: %s",
+ pcap_strerror(errno));
+ }
+ }
+ if (promisc)
+ /* this is allowed to fail */
+ ioctl(fd, BIOCPROMISC, NULL);
+
+ if (ioctl(fd, BIOCGBLEN, ) < 0) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCGBLEN: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+ p->bufsize = v;
+ p->buffer = malloc(p->bufsize);
+ if (p->buffer == NULL) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "malloc: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+ p->activated = 1;
+ return (p);
+
+bad:
+ if (fd >= 0)
+ close(fd);
+ free(p);
+ return (NULL);
+}
+
int
init_pcap(vo
Re: unveil in process accounting and lastcomm
On Thu, Jul 18, 2019 at 04:13:10PM +0200, Alexander Bluhm wrote:
> Hi,
>
> Can we track unveil(2) violators in process accounting lastcomm(1)?
> This makes it easier to find them.
>
> $ lastcomm | grep -e '-[A-Z]U'
> pflogd -FU root__ 0.00 secs Thu Jul 18 14:19 (2:33:22.00)
>
> Seems that pflogd(8) has to be investigated.
Interesting.
This appears to be a false positive, libpcap will first attempt to open
/dev/bpf as O_RDWR and then falls back to O_RDONLY on EACCES. I'm fairly
confident that pflogd(8) does not need write access to bpf. If anything,
unveil(2) appears to have found an old bug here, as before pflogd was
always opening the device with both read/write permissions.
tcpdump avoids this by internalizing more parts of libpcap, and also
opening /dev/bpf O_RDONLY itself.
spamlogd appears to be the only other user of pcap_open_live() in base,
unfortunately it calls unveil after, so /dev/bpf is opened O_RDWR. I
don't use spamlogd, though.
> Also we keep record about programs that may be exploited and do
> something illegal. We have the same mechanism for pledge(2).
>
> Not sure if we want it for both EACCES and ENOENT cases. If it
> creates false positives, we can change that later to EACCES only.
With all that said, I do feel like false positives are incredibly
likely for unveil. For example, I suspect in ports you'll find
software that checks for Linux-isms (/proc, /sys) and and then
fallback. Maybe just EACCES? I don't know.
> ok?
>
> bluhm
>
> Index: kern/kern_unveil.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_unveil.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 kern_unveil.c
> --- kern/kern_unveil.c14 Jul 2019 03:26:02 - 1.27
> +++ kern/kern_unveil.c18 Jul 2019 12:01:24 -
> @@ -18,6 +18,7 @@
>
> #include
>
> +#include
> #include
> #include
> #include
> @@ -823,6 +824,7 @@ unveil_check_final(struct proc *p, struc
> " vnode %p\n",
> p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_vp);
> #endif
> + p->p_p->ps_acflag |= AUNVEIL;
> if (uv->uv_flags & UNVEIL_USERSET)
> return EACCES;
> else
> @@ -865,10 +867,11 @@ unveil_check_final(struct proc *p, struc
>* EACCESS. Otherwise, use any covering match
>* that we found above this dir.
>*/
> - if (uv->uv_flags & UNVEIL_USERSET)
> + if (uv->uv_flags & UNVEIL_USERSET) {
> + p->p_p->ps_acflag |= AUNVEIL;
> return EACCES;
> - else
> - goto done;
> + }
> + goto done;
> }
> /* directory flags match, update match */
> if (uv->uv_flags & UNVEIL_USERSET)
> @@ -881,6 +884,7 @@ unveil_check_final(struct proc *p, struc
> printf("unveil: %s(%d) flag mismatch for terminal '%s'\n",
> p->p_p->ps_comm, p->p_p->ps_pid, tname->un_name);
> #endif
> + p->p_p->ps_acflag |= AUNVEIL;
> return EACCES;
> }
> /* name and flags match in this dir. update match*/
> @@ -903,8 +907,10 @@ done:
> p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_cnd.cn_nameptr,
> ni->ni_unveil_match->uv_vp);
> #endif
> + p->p_p->ps_acflag |= AUNVEIL;
> return EACCES;
> }
> + p->p_p->ps_acflag |= AUNVEIL;
> return ENOENT;
> }
>
> Index: sys/acct.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/acct.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 acct.h
> --- sys/acct.h8 Jun 2017 17:14:02 - 1.7
> +++ sys/acct.h18 Jul 2019 11:37:27 -
> @@ -63,6 +63,7 @@ struct acct {
> #define AXSIG 0x10/* killed by a signal */
> #define APLEDGE 0x20/* killed due to pledge violation */
> #define ATRAP 0x40/* memory access violation */
> +#define AUNVEIL 0x80/* unveil access violation */
> u_int8_t ac_flag; /* accounting flags */
> };
>
>
Re: unveil dhclient (privileged process)
On Wed, Jul 10, 2019 at 03:44:55PM +0100, Ricardo Mestre wrote:
> Hi,
>
> Since the last email I sent, mentioned at the bottom, dhclient(8) has dropped
> support for re-execing itself on SIGHUP and so what path, hardcoded or not,
> should be unveiled for the executable is out of this conversation.
>
> Please check a new diff which unveils /etc/resolv.conf with write/create
> permissions and /etc/resolv.conf.tail with read.
>
> Comments? OK?
>
> Index: dhclient.c
> ===
> RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
> retrieving revision 1.641
> diff -u -p -u -r1.641 dhclient.c
> --- dhclient.c1 Jul 2019 16:53:59 - 1.641
> +++ dhclient.c10 Jul 2019 14:36:31 -
> @@ -2232,6 +2232,13 @@ fork_privchld(struct interface_info *ifi
> if ((routefd = socket(AF_ROUTE, SOCK_RAW, 0)) == -1)
> fatal("socket(AF_ROUTE, SOCK_RAW)");
>
> + if (unveil(_PATH_RESOLV_CONF, "wc") == -1)
> + fatal("unveil");
> + if (unveil(_PATH_RESOLV_CONF_TAIL, "r") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> while (quit == 0) {
> pfd[0].fd = priv_ibuf->fd;
> pfd[0].events = POLLIN;
> Index: dhcpd.h
> ===
> RCS file: /cvs/src/sbin/dhclient/dhcpd.h,v
> retrieving revision 1.278
> diff -u -p -u -r1.278 dhcpd.h
> --- dhcpd.h 22 May 2019 12:56:31 - 1.278
> +++ dhcpd.h 10 Jul 2019 14:36:31 -
> @@ -153,6 +153,8 @@ struct interface_info {
>
> #define _PATH_DHCLIENT_CONF "/etc/dhclient.conf"
> #define _PATH_LEASE_DB "/var/db/dhclient.leases"
> +#define _PATH_RESOLV_CONF "/etc/resolv.conf"
> +#define _PATH_RESOLV_CONF_TAIL "/etc/resolv.conf.tail"
>
> /* options.c */
> int pack_options(unsigned char *, int,
> Index: kroute.c
> ===
> RCS file: /cvs/src/sbin/dhclient/kroute.c,v
> retrieving revision 1.164
> diff -u -p -u -r1.164 kroute.c
> --- kroute.c 30 Jun 2019 19:19:08 - 1.164
> +++ kroute.c 10 Jul 2019 14:36:31 -
> @@ -655,31 +655,31 @@ char *
> resolv_conf_tail(void)
> {
> struct stat sb;
> - const char *tail_path = "/etc/resolv.conf.tail";
> char*tailcontents = NULL;
> ssize_t tailn;
> int tailfd;
>
> - tailfd = open(tail_path, O_RDONLY);
> + tailfd = open(_PATH_RESOLV_CONF_TAIL, O_RDONLY);
> if (tailfd == -1) {
> if (errno != ENOENT)
> - log_warn("%s: open(%s)", log_procname, tail_path);
> + log_warn("%s: open(%s)", log_procname,
> + _PATH_RESOLV_CONF_TAIL);
> } else if (fstat(tailfd, ) == -1) {
> - log_warn("%s: fstat(%s)", log_procname, tail_path);
> + log_warn("%s: fstat(%s)", log_procname, _PATH_RESOLV_CONF_TAIL);
> } else if (sb.st_size > 0 && sb.st_size < LLONG_MAX) {
> tailcontents = calloc(1, sb.st_size + 1);
> if (tailcontents == NULL)
> - fatal("%s contents", tail_path);
> + fatal("%s contents", _PATH_RESOLV_CONF_TAIL);
> tailn = read(tailfd, tailcontents, sb.st_size);
> if (tailn == -1)
> log_warn("%s: read(%s)", log_procname,
> - tail_path);
> + _PATH_RESOLV_CONF_TAIL);
> else if (tailn == 0)
> log_warnx("%s: got no data from %s",
> - log_procname,tail_path);
> + log_procname, _PATH_RESOLV_CONF_TAIL);
> else if (tailn != sb.st_size)
> log_warnx("%s: short read of %s",
> - log_procname, tail_path);
> + log_procname, _PATH_RESOLV_CONF_TAIL);
> else {
> close(tailfd);
> return tailcontents;
> @@ -861,7 +861,6 @@ void
> priv_write_resolv_conf(int index, int routefd, int rdomain, char *contents,
> int *lastidx)
> {
> - const char *path = "/etc/resolv.conf";
> ssize_t n;
> size_t sz;
> int fd, retries, newidx;
> @@ -878,21 +877,21 @@ priv_write_resolv_conf(int index, int ro
> return;
> *lastidx = newidx;
>
> - fd = open(path, O_WRONLY | O_CREAT | O_TRUNC,
> + fd = open(_PATH_RESOLV_CONF, O_WRONLY | O_CREAT | O_TRUNC,
> S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
>
> if (fd == -1) {
> - log_warn("%s: open(%s)", log_procname, path);
> + log_warn("%s: open(%s)", log_procname, _PATH_RESOLV_CONF);
> return;
>
Re: OpenBSD::Unveil perl module
On Sat, Jul 06, 2019 at 03:27:04PM -0700, Andrew Hewus Fresh wrote:
> I wrote up a tiny unveil(2) wrapper for perl, similar to the pledge(2)
> wrapper we have in tree. It passes the tests I wrote, but it's entirely
> possible I'm doing something terrible wrong.
>
> But, I think it could be useful, OK to commit, comments?
I think this is cool, and could be helpful for some perl scripts, same
as OpenBSD::Pledge(3p), perhaps more so.
ok brynet@
> l8rZ,
> --
> andrew - http://afresh1.com
>
> Speed matters.
> Almost as much as some things, and nowhere near as much as others.
> -- Nick Holland
> Index: gnu/usr.bin/perl/MANIFEST
> ===
> RCS file: /tmp/perl/cvs/src/gnu/usr.bin/perl/MANIFEST,v
> retrieving revision 1.52
> diff -u -p -u -p -r1.52 MANIFEST
> --- gnu/usr.bin/perl/MANIFEST 24 May 2019 21:33:50 - 1.52
> +++ gnu/usr.bin/perl/MANIFEST 6 Jul 2019 22:00:52 -
> @@ -1558,6 +1558,9 @@ cpan/OpenBSD-MkTemp/t/OpenBSD-MkTemp.t O
> cpan/OpenBSD-Pledge/lib/OpenBSD/Pledge.pmOpenBSD::Pledge
> cpan/OpenBSD-Pledge/Pledge.xsOpenBSD::Pledge
> cpan/OpenBSD-Pledge/t/OpenBSD-Pledge.t OpenBSD::Pledge test file
> +cpan/OpenBSD-Unveil/lib/OpenBSD/Unveil.pmOpenBSD::Unveil
> +cpan/OpenBSD-Unveil/t/OpenBSD-Unveil.t OpenBSD::Unveil test file
> +cpan/OpenBSD-Unveil/Unveil.xsOpenBSD::Unveil
> cpan/Params-Check/lib/Params/Check.pmParams::Check
> cpan/Params-Check/t/01_Params-Check.tParams::Check tests
> cpan/parent/lib/parent.pmEstablish an ISA relationship
> with base classes at compile time
> Index: gnu/usr.bin/perl/cpan/OpenBSD-Unveil/Unveil.xs
> ===
> RCS file: gnu/usr.bin/perl/cpan/OpenBSD-Unveil/Unveil.xs
> diff -N gnu/usr.bin/perl/cpan/OpenBSD-Unveil/Unveil.xs
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ gnu/usr.bin/perl/cpan/OpenBSD-Unveil/Unveil.xs6 Jul 2019 22:00:53
> -
> @@ -0,0 +1,33 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2019 Andrew Hewus Fresh
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#define PERL_NO_GET_CONTEXT
> +#include "EXTERN.h"
> +#include "perl.h"
> +#include "XSUB.h"
> +
> +#include
> +
> +MODULE = OpenBSD::Unveil PACKAGE = OpenBSD::Unveil
> +
> +int
> +_unveil(const char * path = NULL, const char * permissions = NULL)
> +CODE:
> + RETVAL = unveil(path, permissions) != -1;
> +OUTPUT:
> + RETVAL
> Index: gnu/usr.bin/perl/cpan/OpenBSD-Unveil/lib/OpenBSD/Unveil.pm
> ===
> RCS file: gnu/usr.bin/perl/cpan/OpenBSD-Unveil/lib/OpenBSD/Unveil.pm
> diff -N gnu/usr.bin/perl/cpan/OpenBSD-Unveil/lib/OpenBSD/Unveil.pm
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ gnu/usr.bin/perl/cpan/OpenBSD-Unveil/lib/OpenBSD/Unveil.pm6 Jul
> 2019 22:00:53 -
> @@ -0,0 +1,95 @@
> +#$OpenBSD$ #
> +package OpenBSD::Unveil;
> +
> +use 5.028;
> +use strict;
> +use warnings;
> +
> +use Carp;
> +
> +use parent 'Exporter';
> +our %EXPORT_TAGS = ( 'all' => [qw( unveil )] );
> +our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } );
> +our @EXPORT = qw( unveil ); ## no critic
> 'export'
> +
> +our $VERSION = '0.02';
> +
> +require XSLoader;
> +XSLoader::load( 'OpenBSD::Unveil', $VERSION );
> +
> +sub unveil
> +{ ## no critic 'unpack'
> + croak("Usage: OpenBSD::Unveil::unveil([path, permissions])")
> + unless @_ == 0 || @_ == 2; ## no critic 'postfix'
> + return _unveil(@_);
> +}
> +
> +1;
> +
> +## no critic 'pod sections'
> +__END__
> +
> +=head1 NAME
> +
> +OpenBSD::Unveil - Perl interface to OpenBSD unveil(2)
> +
> +=head1 SYNOPSIS
> +
> + use OpenBSD::Unveil;
> +
> + my $file = "/usr/share/dict/words";
> + unveil( $file, "r" ) || die "Unable to unveil: $!";
> + unveil() || die "Unable to lock unveil: $!";
> + open my $fh, '<', $file or die "Unable to open $file: $!";
> +
> + print grep { /unveil/i } readline($fh);
> + close $fh;
> +
> +
> +=head1 DESCRIPTION
> +
> +This module provides a perl interface to OpenBSD's L
> L.
> +
> +=head1 EXPORT
> +
> +Exports L by default.
Re: [patch] default promises for unprivileged processes
On Sat, Jun 15, 2019 at 04:05:14PM +0200, Srikant T wrote: > + /* XXX cludge to let Xorg function */ > + if (35 == p->p_ucred->cr_uid) > + return; This is completely unacceptable. Kludge is spelled with a K. > --- > END. > >
Re: vmd(8) i8042 device implementation questions
On Thu, May 30, 2019 at 12:09:01AM -0500, Katherine Rohl wrote:
> Okay, here's the first pass of my 8042 device - I wasn't able to figure out
> how to tie the reset line to the guest VM reset, so I was hoping someone
> could give me a hand with that. Other than that, it attaches to i386 and
> amd64 OpenBSD guests. There's no input yet as I mentioned, but the 8042
> commands execute according to specs.
Cool! :-)
Some style(9) nits inline below.
> diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c
> index 4ffb2ff899f..7de38facc78 100644
> --- a/sys/arch/amd64/amd64/vmm.c
> +++ b/sys/arch/amd64/amd64/vmm.c
> @@ -5359,6 +5359,8 @@ svm_handle_inout(struct vcpu *vcpu)
> case IO_ICU1 ... IO_ICU1 + 1:
> case 0x40 ... 0x43:
> case PCKBC_AUX:
> + case 0x60:
> + case 0x64:
> case IO_RTC ... IO_RTC + 1:
> case IO_ICU2 ... IO_ICU2 + 1:
> case 0x3f8 ... 0x3ff:
> diff --git a/usr.sbin/vmd/Makefile b/usr.sbin/vmd/Makefile
> index 8645df7aecf..f39d85b1b14 100644
> --- a/usr.sbin/vmd/Makefile
> +++ b/usr.sbin/vmd/Makefile
> @@ -4,7 +4,7 @@
>
> PROG=vmd
> SRCS=vmd.c control.c log.c priv.c proc.c config.c vmm.c
> -SRCS+= vm.c loadfile_elf.c pci.c virtio.c i8259.c mc146818.c
> +SRCS+= vm.c loadfile_elf.c pci.c virtio.c i8259.c mc146818.c
> i8042.c
> SRCS+= ns8250.c i8253.c vmboot.c ufs.c disklabel.c dhcp.c
> packet.c
> SRCS+= parse.y atomicio.c vioscsi.c vioraw.c vioqcow2.c
> fw_cfg.c
>
> diff --git a/usr.sbin/vmd/i8042.c b/usr.sbin/vmd/i8042.c
> new file mode 100644
> index 000..b57139368cf
> --- /dev/null
> +++ b/usr.sbin/vmd/i8042.c
> @@ -0,0 +1,439 @@
> +/* $OpenBSD: i8042.c,v 1.00 2019/05/25 18:18:00 rohl Exp $ */
This can be just:
/* $OpenBSD$ */
It will be expanded automatically by cvs
> +/*
> + * Copyright (c) 2019 Katherine Rohl
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include
> +
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "i8042.h"
> +#include "proc.h"
> +#include "vmm.h"
> +#include "atomicio.h"
> +#include "vmd.h"
> +
> +struct i8042 {
> + uint8_t data;
> + uint8_t command;
> + uint8_t status;
> + uint8_t internal_ram[16];
> +
> + uint8_t input_port;
> + uint8_t output_port;
> +
> + // Port 0 is keyboard, Port 1 is mouse
C++style single-line comments should be avoided, use /* .. */ instead.
> + uint8_t ps2_enabled[2];
> +
> + uint32_t vm_id;
> +
> + // Is the 8042 awaiting a command argument byte?
...
> + uint8_t awaiting_argbyte;
> +};
> +
> +struct i8042 kbc;
> +pthread_mutex_t kbc_mtx;
> +
> +/*
> + * i8042_init
> + *
> + * Initialize the emulated i8042 KBC.
> + *
> + * Parameters:
> + * vm_id: vmm(4) assigned ID of the VM
> + */
> +void
> +i8042_init(uint32_t vm_id)
> +{
> + memset(, 0, sizeof(kbc));
> +
> + if(pthread_mutex_init(_mtx, NULL) != 0)
> + fatalx("unable to create kbc mutex");
> +
> + kbc.vm_id = vm_id;
> +
> + // Always initialize the reset bit to 1 for proper operation.
..
> + kbc.output_port = KBC_DEFOUTPORT;
> + kbc.input_port = KBC_DEFINPORT;
> +}
> +
> +/*
> + * i8042_set_output_port
> + *
> + * Set the output port of the KBC.
> + * Many bits have side effects, so handle their on/off
> + * transition effects as required.
> + */
> +static void
> +i8042_set_output_port(uint8_t byte)
> +{
> + /* 0x01: reset line
> + * 0x02: A20 gate (not implemented)
> + * 0x04: aux port clock
> + * 0x08: aux port data
> + * 0x10: data in output buffer is from kb port (IRQ1)
> + * 0x20: data in output buffer is from aux port (IRQ12)
> + * 0x40: kb port clock
> + * 0x80: kb port data
> + */
> +
> + uint8_t old_output_port = kbc.output_port;
> + kbc.output_port = byte;
> +
> + // 0 -> 1 transition
..
The braces should be on the same line, or in cases like this can
be omitted. There should also be a space after keywords, if (..).
> + if(((old_output_port & 0x10) == 0) &&
> +((kbc.output_port & 0x10) == 1))
> +{
> +vcpu_assert_pic_irq(kbc.vm_id, 0,
Re: remove mention of YP from hosts(5)
On Sun, May 26, 2019 at 03:57:48PM +0200, Jan-Piet Mens wrote: > I noticed hosts(5) still mentions YP in spite of support for the latter > having been removed in [1]. This minuscule patch corrects the man page. > > Index: share/man/man5/hosts.5 > === > RCS file: /cvs/src/share/man/man5/hosts.5,v > retrieving revision 1.24 > diff -u -p -u -r1.24 hosts.5 > --- share/man/man5/hosts.530 Apr 2018 18:01:24 - 1.24 > +++ share/man/man5/hosts.526 May 2019 13:40:20 - > @@ -58,7 +58,6 @@ The system configuration file > controls where host name information will be searched for. > The mechanism provided permits the administrator to describe the > databases to search; the databases currently known include > -.Xr yp 8 , > DNS > and the > .Nm hosts > > > [1] https://marc.info/?l=openbsd-cvs=144830197501354=2 Nice find! ok brynet@
Re: [patch] ex/vi(1): wait with pledge until *after* ~/.nexrc is read
On Tue, May 21, 2019 at 07:34:05AM +0200, Martijn van Duren wrote:
> Hello Jesper,
> On 5/20/19 10:58 PM, Jesper Wallin wrote:
> > Hi all,
> >
> > When ex/vi is started with -S (secure), a stricter pledge is used to
> > prevent exec from being used. It's tedious to specify -S all the time
> > and easier to add "set secure" to ~/.nexrc. However, the check for
> > which pledge to use doesn't care what your ~/.nexrc contains and the
> > exec promise remains.
>
> The behaviour should be identical, the only difference would be that
> pledge catches programming errors. So I see no particular reason to use
> -S over "set secure" for normal users; even without pledge.
> >
> > This patch simply wait until the ~/.nexrc is parsed and all options are
> > set before checking whether or not to apply the stricter pledge.
> >
> > Another approach would be to also have a check inside the opts_set()
> > unction, in case the user manually runs "set secure", but that feels
> > ugly and "too deep".
> >
> If we want to make sure that that secure is always honoured with a
> pledge I reckon we should push it down to opts_set.
> I choose not to fail hard on pledge, since that could loose peoples
> work, which is most definitely not what we want. While here fix a
> lie that secure has an off parameter and inform the user that it can't
> be turned off again.
>
> OK?
Makes sense to me. ok brynet@
> martijn@
> >
> > Jesper Wallin
> >
> Index: common/options.c
> ===
> RCS file: /cvs/src/usr.bin/vi/common/options.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 options.c
> --- common/options.c 31 Jul 2017 19:45:49 - 1.26
> +++ common/options.c 21 May 2019 05:32:29 -
> @@ -136,7 +136,7 @@ OPTLIST const optlist[] = {
> /* O_SECTIONS4BSD */
> {"sections",f_section, OPT_STR,0},
> /* O_SECURE4.4BSD */
> - {"secure", NULL, OPT_0BOOL, OPT_NOUNSET},
> + {"secure", f_secure, OPT_0BOOL, OPT_NOUNSET},
> /* O_SHELL 4BSD */
> {"shell", NULL, OPT_STR,0},
> /* O_SHELLMETA 4.4BSD */
> Index: common/options_f.c
> ===
> RCS file: /cvs/src/usr.bin/vi/common/options_f.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 options_f.c
> --- common/options_f.c3 Jul 2017 07:01:14 - 1.12
> +++ common/options_f.c21 May 2019 05:32:30 -
> @@ -207,6 +207,19 @@ f_section(SCR *sp, OPTION *op, char *str
> }
>
> /*
> + * PUBLIC: int f_secure(SCR *, OPTION *, char *, u_long *)
> + */
> +int
> +f_secure(SCR *sp, OPTION *op, char *str, u_long *valp)
> +{
> + if (pledge("stdio rpath wpath cpath fattr flock getpw tty", NULL) ==
> -1) {
> + msgq(sp, M_ERR, "pledge failed");
> + return (1);
> + }
> + return (0);
> +}
> +
> +/*
> * PUBLIC: int f_ttywerase(SCR *, OPTION *, char *, u_long *);
> */
> int
> Index: docs/USD.doc/vi.man/vi.1
> ===
> RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
> retrieving revision 1.75
> diff -u -p -r1.75 vi.1
> --- docs/USD.doc/vi.man/vi.1 12 Feb 2018 01:10:46 - 1.75
> +++ docs/USD.doc/vi.man/vi.1 21 May 2019 05:32:30 -
> @@ -2456,8 +2456,9 @@ Define additional section boundaries for
> and
> .Cm ]]
> commands.
> -.It Cm secure Bq off
> +.It Cm secure
> Turns off all access to external programs.
> +Once set this option can't be disabled.
> .It Cm shell , sh Bq "environment variable SHELL, or /bin/sh"
> Select the shell used by the editor.
> .It Cm shellmeta Bq ~{[*?$`'\&"\e
> Index: include/com_extern.h
> ===
> RCS file: /cvs/src/usr.bin/vi/include/com_extern.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 com_extern.h
> --- include/com_extern.h 3 Jul 2017 07:01:14 - 1.15
> +++ include/com_extern.h 21 May 2019 05:32:30 -
> @@ -75,6 +75,7 @@ int f_readonly(SCR *, OPTION *, char *,
> int f_recompile(SCR *, OPTION *, char *, u_long *);
> int f_reformat(SCR *, OPTION *, char *, u_long *);
> int f_section(SCR *, OPTION *, char *, u_long *);
> +int f_secure(SCR *, OPTION *, char *, u_long *);
> int f_ttywerase(SCR *, OPTION *, char *, u_long *);
> int f_w300(SCR *, OPTION *, char *, u_long *);
> int f_w1200(SCR *, OPTION *, char *, u_long *);
>
>
Re: unveil tcpdrop
On Tue, Apr 30, 2019 at 06:23:57PM +0100, Ricardo Mestre wrote:
> Went through my old sent emails and saw this one still pending on my tree.
>
> Is this OK?
>
> On 13:02 Wed 07 Nov , Ricardo Mestre wrote:
> > Hi,
> >
> > tcpdrop(8) needs to access only two files, in this case /etc/hosts and
> > /etc/resolv.conf both with read permissions for the purpose of name
> > resolution.
> > ethers(5) is not needed since we are not using any of the ether_*(3) family.
> >
> > Since unistd.h needs to be included I also shuffled netdb.h into the right
> > place.
> >
> > Comments? OK?
> >
> > Index: tcpdrop.c
> > ===
> > RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
> > retrieving revision 1.17
> > diff -u -p -u -r1.17 tcpdrop.c
> > --- tcpdrop.c 16 Jan 2015 06:40:21 - 1.17
> > +++ tcpdrop.c 6 Nov 2018 10:48:10 -
> > @@ -27,10 +27,11 @@
> > #include
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > -#include
> > +#include
> >
> > __dead void usage(void);
> >
> > @@ -61,6 +62,13 @@ main(int argc, char **argv)
> > char *laddr1, *addr1, *port1, *faddr2, *addr2, *port2;
> > struct tcp_ident_mapping tir;
> > int gaierr, rval = 0;
> > +
> > + if (unveil("/etc/hosts", "r") == -1)
> > + err(1, "unveil");
> > + if (unveil("/etc/resolv.conf", "r") == -1)
> > + err(1, "unveil");
> > + if (unveil(NULL, NULL) == -1)
> > + err(1, "unveil");
> >
> > memset(, 0, sizeof(hints));
> > hints.ai_family = AF_UNSPEC;
This seems low risk, and looks good to me also.
ok brynet@
Re: acme-client.1: update STANDARDS
On Wed, Apr 24, 2019 at 03:08:59PM +0200, Fabio Scotoni wrote: > This diff updates the acme-client(1) STANDARDS section. > Currently, it lists an RFC draft for the ACME protocol. > Since March of this year, there is a proposed standard with an actual > RFC number. > > While at it, make the format match ssh(1) STANDARDS by providing .%A and > .%D entries. > > Index: usr.sbin/acme-client/acme-client.1 > === > RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v > retrieving revision 1.29 > diff -u -p -u -p -r1.29 acme-client.1 > --- usr.sbin/acme-client/acme-client.1 3 Feb 2019 20:39:35 - 1.29 > +++ usr.sbin/acme-client/acme-client.1 24 Apr 2019 13:05:10 - > @@ -145,7 +145,12 @@ is reloaded: > .Xr httpd.conf 5 > .Sh STANDARDS > .Rs > -.%U https://tools.ietf.org/html/draft-ietf-acme-acme-03 > +.%A R. Barnes > +.%A J. Hoffman-Andrews > +.%A D. McCarney > +.%A J. Kasten > +.%D March 2019 > +.%R RFC 8555 > .%T Automatic Certificate Management Environment (ACME) > .Re > .Sh HISTORY Isn't RF C8555 ACMEv2? acme-client(1) only supports ACMEv1, so I don't think this is correct.
Re: Booting Threadripper 2950x with -current
On Mon, Apr 22, 2019 at 10:06:44PM +, Bryan Everly wrote: > Hi @tech, > > I just got through building a new desktop machine and thought I'd > install OpenBSD -current on it. The install kernel booted quite fast, > but now that I have the real kernel there, it takes approximately 5 > minutes to boot. The vast majority of the time is spent at the very > beginning where the "spinning text graphic" sits there until it kicks > out a number and then does the next one (not sure what to call that). That sounds like the part where it's loading the kernel from the disk, which is using BIOS/UEFI functions. Not sure if there's much all that can be done about that. :) > How could I provide appropriate info to help debug this and get a fix > in? I'm guessing that not too many people are running on this CPU. By > the way, all 32 cores show up on it! > 16 cores, half of them are threads. You can disable SMT in the BIOS. ;-) > Thanks. > >
Re: httpd(8): Adapt to industry wide current best security practices
On Mon, Apr 01, 2019 at 02:30:22AM +0200, Florian Obser wrote:
> OK?
>
> diff --git server_http.c server_http.c
> index 6c8549d2b41..f04a15bd056 100644
> --- server_http.c
> +++ server_http.c
> @@ -1176,7 +1176,7 @@ server_response(struct httpd *httpd, struct client *clt)
> struct http_descriptor *resp = clt->clt_descresp;
> struct server *srv = clt->clt_srv;
> struct server_config*srv_conf = >srv_conf;
> - struct kv *kv, key, *host;
> + struct kv *kv, key, *host, *ua;
> struct str_find sm;
> int portval = -1, ret;
> char*hostval, *query;
> @@ -1194,6 +1194,15 @@ server_response(struct httpd *httpd, struct client
> *clt)
> if ((desc->http_path = strdup(path)) == NULL)
> goto fail;
>
> + key.kv_key = "user-agent";
I think this should be "User-Agent" to match the other cases in
the file.
With that change, ok brynet@
> + if ((ua = kv_find(>http_headers, )) != NULL &&
> + ua->kv_value != NULL) {
> + if (strstr(ua->kv_value, "curl") != NULL) {
> + server_abort_http(clt, 403, "forbidden");
> + return (-1);
> + }
> + }
> +
> key.kv_key = "Host";
> if ((host = kv_find(>http_headers, )) != NULL &&
> host->kv_value == NULL)
>
> --
> I'm not entirely sure you are real.
>
>
Re: unveil file(1)
On Fri, Jan 04, 2019 at 11:52:05AM -0500, Ted Unangst wrote:
> Theo de Raadt wrote:
> > > unveil isn't really buying much if you pledge "rpath" immediately after,
> > > so if you want just add another pledge here instead, that is fine.
> >
> > "rpath" is obviously cheaper than unveil of even 1 file.
>
> here is a diff that simply adds another pledge.
>
> the attack surface here is kinda nonexistant, but no reason why it needs the
> ability to write files either.
>
> Index: file.c
> ===
> RCS file: /cvs/src/usr.bin/file/file.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 file.c
> --- file.c15 Jan 2018 19:45:51 - 1.66
> +++ file.c4 Jan 2019 16:50:11 -
> @@ -168,6 +168,9 @@ main(int argc, char **argv)
> } else if (argc == 0)
> usage();
>
> + if (pledge("stdio rpath getpw recvfd sendfd id proc", NULL) == -1)
> + err(1, "pledge");
> +
> magicfp = NULL;
> if (geteuid() != 0 && !issetugid()) {
> home = getenv("HOME");
ok brynet@
Re: unveil file(1)
On Thu, Jan 03, 2019 at 08:26:00PM -0500, Ted Unangst wrote:
> Ted Unangst wrote:
> > Bryan Steele wrote:
> > > It is not possible to unveil(2) all arguments passed to file(1), as this
> > > would require walking *argv. Instead, we can unveil("/", "r") to permit
> > > readonly access to the entire filesystem, while restricting all execute
> > > write, and create operations.
> >
> > Why not? Because of the limit? We can still try unveil up to a certain
> > limit.
> >
> > > This only provides some additional early protection for the parent, as
> > > the privsep magic(5) parser already pledged tightly.
> > >
> > > It might be possible to use pledge instead, but this since this process
> > > doesn't do much more than opening files and passing descriptors, unveil
> > > alone should be enough..
> >
> > I think if we want to enforce read only access, pledge is still the way to
> > go.
> >
> > This seems to work.
>
> oops, forgot the error checking for some unveil calls. this is better.
>
>
> Index: file.c
> ===
> RCS file: /cvs/src/usr.bin/file/file.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 file.c
> --- file.c15 Jan 2018 19:45:51 - 1.66
> +++ file.c4 Jan 2019 01:24:47 -
> @@ -168,6 +168,19 @@ main(int argc, char **argv)
> } else if (argc == 0)
> usage();
>
> + if (argc < 64) {
> + if (unveil("/etc/magic", "r") == -1)
> + err(1, "unveil");
> + for (idx = 0; idx < argc; idx++)
> + if (unveil(argv[idx], "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> + }
> +
Agreeing with what Theo said..
It is actually common to run file(1) on directories with a lot of files,
i.e: file *|grep. espie@ hit this is in ports when I ade the similar
mistake of trying to pre-open files.
unveil isn't really buying much if you pledge "rpath" immediately after,
so if you want just add another pledge here instead, that is fine.
> + if (pledge("stdio rpath getpw recvfd sendfd id proc", NULL) == -1)
> + err(1, "pledge");
> +
> magicfp = NULL;
> if (geteuid() != 0 && !issetugid()) {
> home = getenv("HOME");
unveil file(1)
It is not possible to unveil(2) all arguments passed to file(1), as this
would require walking *argv. Instead, we can unveil("/", "r") to permit
readonly access to the entire filesystem, while restricting all execute
write, and create operations.
This only provides some additional early protection for the parent, as
the privsep magic(5) parser already pledged tightly.
It might be possible to use pledge instead, but this since this process
doesn't do much more than opening files and passing descriptors, unveil
alone should be enough..
Needs the recent unveil(2) commit in -current by Bob Beck.
ok?
Index: file.c
===
RCS file: /cvs/src/usr.bin/file/file.c,v
retrieving revision 1.66
diff -u -p -u -r1.66 file.c
--- usr.bin/file/file.c 15 Jan 2018 19:45:51 - 1.66
+++ usr.bin/file/file.c 3 Jan 2019 23:07:41 -
@@ -168,6 +168,11 @@ main(int argc, char **argv)
} else if (argc == 0)
usage();
+ if (unveil("/", "r") == -1)
+ err(1, "unveil");
+ if (unveil(NULL, NULL) == -1)
+ err(1, "unveil");
+
magicfp = NULL;
if (geteuid() != 0 && !issetugid()) {
home = getenv("HOME");
tcpdump(8) monitor privdrop
tcpdump's privsep monitor process handles any privileged operations on
behalf of the unprivileged "packet parser" process. After this, it
enters its final runtime state, which:
* Performs DNS and other "numbers to names" lookups, sending results
back over a pipe/socketpair.
* Displays the final packet statistics on ^C.
We can now go a step further by dropping root privileges here, as bpf
BIOCGSTATS is still permitted by non-root on open descriptors after it
has been permanently locked with BIOCLOCK. This provides some additional
protection, to go along with the already tight unveil(2) and pledge(2)
restrictions.
This also fixes a small corner case with '-w' mode, as the monitor
process may not always finish with PRIV_INIT_DONE. I don't really
think this was a problem (in this case no DNS lookups are performed,
only ^C statistics).
comments or ok? :-)
-Bryan.
Index: privsep.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.51
diff -u -p -u -r1.51 privsep.c
--- usr.sbin/tcpdump/privsep.c 9 Nov 2018 18:39:34 - 1.51
+++ usr.sbin/tcpdump/privsep.c 16 Nov 2018 17:27:37 -
@@ -102,6 +102,8 @@ static volatile sig_atomic_t cur_state =
extern voidset_slave_signals(void);
+static voiddrop_privs(int);
+
static voidimpl_open_bpf(int, int *);
static voidimpl_open_dump(int, const char *);
static voidimpl_open_pfosfp(int);
@@ -119,11 +121,42 @@ static void impl_pcap_stats(int, int *);
static voidtest_state(int, int);
static voidlogmsg(int, const char *, ...);
+static void
+drop_privs(int nochroot)
+{
+ struct passwd *pw;
+
+ /*
+* If run as regular user, then tcpdump will rely on
+* pledge(2). If we are root, we want to chroot also..
+*/
+ if (getuid() != 0)
+ return;
+
+ pw = getpwnam("_tcpdump");
+ if (pw == NULL)
+ errx(1, "unknown user _tcpdump");
+
+ if (!nochroot) {
+ if (chroot(pw->pw_dir) == -1)
+ err(1, "unable to chroot");
+ if (chdir("/") == -1)
+ err(1, "unable to chdir");
+ }
+
+ /* drop to _tcpdump */
+ if (setgroups(1, >pw_gid) == -1)
+ err(1, "setgroups() failed");
+ if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1)
+ err(1, "setresgid() failed");
+ if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1)
+ err(1, "setresuid() failed");
+}
+
int
priv_init(int argc, char **argv)
{
int i, nargc, socks[2];
- struct passwd *pw;
sigset_t allsigs, oset;
char **privargv;
@@ -149,29 +182,7 @@ priv_init(int argc, char **argv)
set_slave_signals();
sigprocmask(SIG_SETMASK, , NULL);
- /*
-* If run as regular user, packet parser will rely on
-* pledge(2). If we are root, we want to chroot also..
-*/
- if (getuid() != 0)
- return (0);
-
- pw = getpwnam("_tcpdump");
- if (pw == NULL)
- errx(1, "unknown user _tcpdump");
-
- if (chroot(pw->pw_dir) == -1)
- err(1, "unable to chroot");
- if (chdir("/") == -1)
- err(1, "unable to chdir");
-
- /* drop to _tcpdump */
- if (setgroups(1, >pw_gid) == -1)
- err(1, "setgroups() failed");
- if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1)
- err(1, "setresgid() failed");
- if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1)
- err(1, "setresuid() failed");
+ drop_privs(0);
return (0);
}
@@ -256,10 +267,8 @@ priv_exec(int argc, char *argv[])
if (WFileName != NULL) {
if (strcmp(WFileName, "-") != 0)
allowed_ext[STATE_FILTER] |= ALLOW(PRIV_OPEN_OUTPUT);
- else
- allowed_ext[STATE_FILTER] |= ALLOW(PRIV_INIT_DONE);
- } else
- allowed_ext[STATE_FILTER] |= ALLOW(PRIV_INIT_DONE);
+ }
+ allowed_ext[STATE_FILTER] |= ALLOW(PRIV_INIT_DONE);
if (!nflag) {
allowed_ext[STATE_RUN] |= ALLOW(PRIV_GETHOSTBYADDR);
allowed_ext[STATE_FILTER] |= ALLOW(PRIV_ETHER_NTOHOST);
@@ -294,7 +303,7 @@ priv_exec(int argc, char *argv[])
impl_open_pfosfp(sock);
break;
case PRIV_OPEN_OUTPUT:
- test_state(cmd, STATE_RUN);
+ test_state(cmd, STATE_FILTER);
impl_open_output(sock, WFileName);
break;
case PRIV_SETFILTER:
@@ -305,6 +314,7 @@ priv_exec(int
Re: YP/NIS support in /etc/ethers, libc ether_ntohost/ether_hostton
This was suggested by deraadt@, sorry.
On Thu, Nov 08, 2018 at 08:05:13PM -0500, Bryan Steele wrote:
> These libc functions are used to map hardware MAC addresses to hostnames
> and vice versa. If it exists, /etc/ethers will typically contain a
> number of lines like so:
>
> 34:00:8a:56:10:20 superman
>
> In addition to that, there is support for using a YP (nee Yellow Pee)
> lookup service:
>
> "If a '+' appears alone on a line in the file, then ether_hostton() will
> consult the x ethers.byname YP map, and ether_ntohost() will consult the
> ethers.byaddr YP map."
>
> This support currently interferes with my work to reduce the pledge(2)
> in tcpdump(8), as the "inet" promise is required to perform these
> lookups..
>
> I've come up with small a diff to remove it, but it was suggested there
> may be some interactions with ldap, and I'm not sure how important this
> functionality may be to existing YP users (I am not one).
>
> Any objections to this approach? (Missing man page removal bits)
>
> -Bryan.
>
> Index: ethers.c
> ===
> RCS file: /cvs/src/lib/libc/net/ethers.c,v
> retrieving revision 1.25
> diff -u -p -u -r1.25 ethers.c
> --- lib/libc/net/ethers.c 21 Sep 2016 04:38:56 - 1.25
> +++ lib/libc/net/ethers.c 8 Nov 2018 23:54:19 -
> @@ -34,9 +34,6 @@
> #include
> #include
> #include
> -#ifdef YP
> -#include
> -#endif
>
> #ifndef _PATH_ETHERS
> #define _PATH_ETHERS "/etc/ethers"
> @@ -99,18 +96,6 @@ ether_ntohost(char *hostname, struct eth
> char buf[BUFSIZ+1], *p;
> size_t len;
> struct ether_addr try;
> -#ifdef YP
> - char trybuf[sizeof("xx:xx:xx:xx:xx:xx")];
> - int trylen;
> -#endif
> -
> -#ifdef YP
> - snprintf(trybuf, sizeof trybuf, "%x:%x:%x:%x:%x:%x",
> - e->ether_addr_octet[0], e->ether_addr_octet[1],
> - e->ether_addr_octet[2], e->ether_addr_octet[3],
> - e->ether_addr_octet[4], e->ether_addr_octet[5]);
> - trylen = strlen(trybuf);
> -#endif
>
> f = fopen(_PATH_ETHERS, "re");
> if (f == NULL)
> @@ -123,26 +108,9 @@ ether_ntohost(char *hostname, struct eth
> (void)memcpy(buf, p, len);
> buf[len] = '\n';/* code assumes newlines later on */
> buf[len+1] = '\0';
> -#ifdef YP
> - /* A + in the file means try YP now. */
> - if (!strncmp(buf, "+\n", sizeof(buf))) {
> - char *ypbuf, *ypdom;
> - int ypbuflen;
> -
> - if (yp_get_default_domain())
> - continue;
> - if (yp_match(ypdom, "ethers.byaddr", trybuf,
> - trylen, , ))
> - continue;
> - if (ether_line(ypbuf, , hostname) == 0) {
> - free(ypbuf);
> - (void)fclose(f);
> - return (0);
> - }
> - free(ypbuf);
> + /* A + in the file meant try YP, ignore it. */
> + if (!strncmp(buf, "+\n", sizeof(buf)))
> continue;
> - }
> -#endif
> if (ether_line(buf, , hostname) == 0 &&
> memcmp(, e, sizeof(try)) == 0) {
> (void)fclose(f);
> @@ -161,9 +129,6 @@ ether_hostton(const char *hostname, stru
> char buf[BUFSIZ+1], *p;
> char try[HOST_NAME_MAX+1];
> size_t len;
> -#ifdef YP
> - int hostlen = strlen(hostname);
> -#endif
>
> f = fopen(_PATH_ETHERS, "re");
> if (f==NULL)
> @@ -177,26 +142,9 @@ ether_hostton(const char *hostname, stru
> memcpy(buf, p, len);
> buf[len] = '\n';/* code assumes newlines later on */
> buf[len+1] = '\0';
> -#ifdef YP
> - /* A + in the file means try YP now. */
> - if (!strncmp(buf, "+\n", sizeof(buf))) {
> - char *ypbuf, *ypdom;
> - int ypbuflen;
> -
> - if (yp_get_default_domain())
> - continue;
> - if (yp_match(ypdom, "ethers.byname", hostname, hostlen,
> - , ))
> - continue;
> - if (ether_line(ypbuf, e, try) == 0) {
> - free(ypbuf);
> - (void)fclose(f);
> - return (0);
> - }
> - free(ypbuf);
> + /* A + in the file meant try YP, ignore it. */
> + if (!strncmp(buf, "+\n", sizeof(buf)))
> continue;
> - }
> -#endif
> if (ether_line(buf, e, try) == 0 && strcmp(hostname, try) == 0)
> {
> (void)fclose(f);
> return (0);
YP/NIS support in /etc/ethers, libc ether_ntohost/ether_hostton
These libc functions are used to map hardware MAC addresses to hostnames
and vice versa. If it exists, /etc/ethers will typically contain a
number of lines like so:
34:00:8a:56:10:20 superman
In addition to that, there is support for using a YP (nee Yellow Pee)
lookup service:
"If a '+' appears alone on a line in the file, then ether_hostton() will
consult the x ethers.byname YP map, and ether_ntohost() will consult the
ethers.byaddr YP map."
This support currently interferes with my work to reduce the pledge(2)
in tcpdump(8), as the "inet" promise is required to perform these
lookups..
I've come up with small a diff to remove it, but it was suggested there
may be some interactions with ldap, and I'm not sure how important this
functionality may be to existing YP users (I am not one).
Any objections to this approach? (Missing man page removal bits)
-Bryan.
Index: ethers.c
===
RCS file: /cvs/src/lib/libc/net/ethers.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 ethers.c
--- lib/libc/net/ethers.c 21 Sep 2016 04:38:56 - 1.25
+++ lib/libc/net/ethers.c 8 Nov 2018 23:54:19 -
@@ -34,9 +34,6 @@
#include
#include
#include
-#ifdef YP
-#include
-#endif
#ifndef _PATH_ETHERS
#define _PATH_ETHERS "/etc/ethers"
@@ -99,18 +96,6 @@ ether_ntohost(char *hostname, struct eth
char buf[BUFSIZ+1], *p;
size_t len;
struct ether_addr try;
-#ifdef YP
- char trybuf[sizeof("xx:xx:xx:xx:xx:xx")];
- int trylen;
-#endif
-
-#ifdef YP
- snprintf(trybuf, sizeof trybuf, "%x:%x:%x:%x:%x:%x",
- e->ether_addr_octet[0], e->ether_addr_octet[1],
- e->ether_addr_octet[2], e->ether_addr_octet[3],
- e->ether_addr_octet[4], e->ether_addr_octet[5]);
- trylen = strlen(trybuf);
-#endif
f = fopen(_PATH_ETHERS, "re");
if (f == NULL)
@@ -123,26 +108,9 @@ ether_ntohost(char *hostname, struct eth
(void)memcpy(buf, p, len);
buf[len] = '\n';/* code assumes newlines later on */
buf[len+1] = '\0';
-#ifdef YP
- /* A + in the file means try YP now. */
- if (!strncmp(buf, "+\n", sizeof(buf))) {
- char *ypbuf, *ypdom;
- int ypbuflen;
-
- if (yp_get_default_domain())
- continue;
- if (yp_match(ypdom, "ethers.byaddr", trybuf,
- trylen, , ))
- continue;
- if (ether_line(ypbuf, , hostname) == 0) {
- free(ypbuf);
- (void)fclose(f);
- return (0);
- }
- free(ypbuf);
+ /* A + in the file meant try YP, ignore it. */
+ if (!strncmp(buf, "+\n", sizeof(buf)))
continue;
- }
-#endif
if (ether_line(buf, , hostname) == 0 &&
memcmp(, e, sizeof(try)) == 0) {
(void)fclose(f);
@@ -161,9 +129,6 @@ ether_hostton(const char *hostname, stru
char buf[BUFSIZ+1], *p;
char try[HOST_NAME_MAX+1];
size_t len;
-#ifdef YP
- int hostlen = strlen(hostname);
-#endif
f = fopen(_PATH_ETHERS, "re");
if (f==NULL)
@@ -177,26 +142,9 @@ ether_hostton(const char *hostname, stru
memcpy(buf, p, len);
buf[len] = '\n';/* code assumes newlines later on */
buf[len+1] = '\0';
-#ifdef YP
- /* A + in the file means try YP now. */
- if (!strncmp(buf, "+\n", sizeof(buf))) {
- char *ypbuf, *ypdom;
- int ypbuflen;
-
- if (yp_get_default_domain())
- continue;
- if (yp_match(ypdom, "ethers.byname", hostname, hostlen,
- , ))
- continue;
- if (ether_line(ypbuf, e, try) == 0) {
- free(ypbuf);
- (void)fclose(f);
- return (0);
- }
- free(ypbuf);
+ /* A + in the file meant try YP, ignore it. */
+ if (!strncmp(buf, "+\n", sizeof(buf)))
continue;
- }
-#endif
if (ether_line(buf, e, try) == 0 && strcmp(hostname, try) == 0)
{
(void)fclose(f);
return (0);
Re: tcpdump: revisiting some old diffs, remove unused pledges
On Wed, Nov 07, 2018 at 07:32:25PM -0500, Bryan Steele wrote:
> On Wed, Nov 07, 2018 at 07:06:09PM -0500, Bryan Steele wrote:
> > I'm revisiting some old tcpdump diffs, now that mestre@ has added proper
> > unveil(2) support! :-)
> >
> > Refresher: https://marc.info/?l=openbsd-tech=150535073209723=2
> >
> > This hoists opening pf.os(5) fingerprints '-o' from the 'RUN' state to
> > the 'FILTER' state, this will allow for a reduced pledge(2) at runtime
> > in the (currently root) monitor process.
>
> This was a bit of copy & paste, sorry. This moves the opening of pf.os
> earlier and avoids the unveil later on. Of course, reducing the runtime
> pledge(2) promises will come later! :-)
>
> >
> > This still works as well as it already has. :-)
> >
> > ( ... ) [tcp sum ok] (src OS: OpenBSD 6.1) 3311509932:3311509932(0) win
> > 16384
> > (DF) (ttl 64, id 41239, len 64)
> >
> > The only potential difference is that if /etc/pf.os is replaced at
> > runtime, tcpdump won't reopen it.
> >
> > I don't think that's a problem..
> >
> > ok?
> >
> > -Bryan.
>
The first two diffs are in now. Thanks!
The "recvfd" promise doesn't appear to be used by the privileged
monitor process, which currently only handles resolving domain names,
and displaying BIOCGSTATS on ^C. It never sends or receives any file
descriptors while in the 'RUN' state. We can drop it. :-)
Unfortunately the "inet" promise appears to be necessary for some yp(8)
environments, as documented in ether_ntohost(3). I'm not familiar enough
with YP to know if this special /etc/ethers '+' line is typical, or if
DNS should be used instead? In that case it would be covered by the
existing "dns" promise and we could drop "inet" here.
I'd like to try this, and hopefully there's a better solution for YP &
pledge(2) later?
comments or ok?
-Bryan.
Index: privsep.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.50
diff -u -p -u -r1.50 privsep.c
--- usr.sbin/tcpdump/privsep.c 8 Nov 2018 14:06:09 - 1.50
+++ usr.sbin/tcpdump/privsep.c 8 Nov 2018 18:43:06 -
@@ -309,7 +309,7 @@ priv_exec(int argc, char *argv[])
err(1, "unveil");
if (unveil("/etc/rpc", "r") == -1)
err(1, "unveil");
- if (pledge("stdio rpath inet dns recvfd bpf", NULL) ==
-1)
+ if (pledge("stdio rpath dns bpf", NULL) == -1)
err(1, "pledge");
break;
Re: tcpdump: revisiting some old diffs, cleanup unused functions
On Wed, Nov 07, 2018 at 07:06:09PM -0500, Bryan Steele wrote:
> I'm revisiting some old tcpdump diffs, now that mestre@ has added proper
> unveil(2) support! :-)
>
> Refresher: https://marc.info/?l=openbsd-tech=150535073209723=2
>
> This hoists opening pf.os(5) fingerprints '-o' from the 'RUN' state to
> the 'FILTER' state, this will allow for a reduced pledge(2) at runtime
> in the (currently root) monitor process.
This was a bit of copy & paste, sorry. This moves the opening of pf.os
earlier and avoids the unveil later on. Of course, reducing the runtime
pledge(2) promises will come later! :-)
>
> This still works as well as it already has. :-)
>
> ( ... ) [tcp sum ok] (src OS: OpenBSD 6.1) 3311509932:3311509932(0) win
> 16384
> (DF) (ttl 64, id 41239, len 64)
>
> The only potential difference is that if /etc/pf.os is replaced at
> runtime, tcpdump won't reopen it.
>
> I don't think that's a problem..
>
> ok?
>
> -Bryan.
Remove the now unused internal privsep "getline" code, which passed
lines over a socket, replaced with explicit fdpassing of /etc/pf.os.
This depends on the previous diff..
ok?
-Bryan.
Index: privsep.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.49
diff -u -p -u -r1.49 privsep.c
--- privsep.c 28 Sep 2018 06:48:59 - 1.49
+++ privsep.c 8 Nov 2018 00:19:47 -
@@ -77,8 +77,8 @@ static const int allowed_max[] = {
ALLOW(PRIV_GETPROTOENTRIES) |
ALLOW(PRIV_ETHER_NTOHOST) | ALLOW(PRIV_INIT_DONE),
/* RUN */ ALLOW(PRIV_GETHOSTBYADDR) | ALLOW(PRIV_ETHER_NTOHOST) |
- ALLOW(PRIV_GETRPCBYNUMBER) | ALLOW(PRIV_GETLINES) |
- ALLOW(PRIV_LOCALTIME) | ALLOW(PRIV_PCAP_STATS),
+ ALLOW(PRIV_GETRPCBYNUMBER) | ALLOW(PRIV_LOCALTIME) |
+ ALLOW(PRIV_PCAP_STATS),
/* EXIT */ 0
};
@@ -90,21 +90,10 @@ static int allowed_ext[] = {
/* INIT */ ALLOW(PRIV_SETFILTER),
/* BPF */ ALLOW(PRIV_SETFILTER),
/* FILTER */ALLOW(PRIV_GETSERVENTRIES),
- /* RUN */ ALLOW(PRIV_GETLINES) | ALLOW(PRIV_LOCALTIME) |
- ALLOW(PRIV_PCAP_STATS),
+ /* RUN */ ALLOW(PRIV_LOCALTIME) | ALLOW(PRIV_PCAP_STATS),
/* EXIT */ 0
};
-struct ftab {
- char *name;
- int max;
- int count;
-};
-
-static struct ftab file_table[] = {{PF_OSFP_FILE, 1, 0}};
-
-#define NUM_FILETAB (sizeof(file_table) / sizeof(struct ftab))
-
intdebug_level = LOG_INFO;
intpriv_fd = -1;
volatile pid_t child_pid = -1;
@@ -123,7 +112,6 @@ static void impl_getrpcbynumber(int);
static voidimpl_getserventries(int);
static voidimpl_getprotoentries(int);
static voidimpl_localtime(int fd);
-static voidimpl_getlines(int);
static voidimpl_pcap_stats(int, int *);
static voidtest_state(int, int);
@@ -345,10 +333,6 @@ priv_exec(int argc, char *argv[])
test_state(cmd, STATE_RUN);
impl_localtime(sock);
break;
- case PRIV_GETLINES:
- test_state(cmd, STATE_RUN);
- impl_getlines(sock);
- break;
case PRIV_PCAP_STATS:
test_state(cmd, STATE_RUN);
impl_pcap_stats(sock, );
@@ -577,55 +561,6 @@ impl_localtime(int fd)
}
static void
-impl_getlines(int fd)
-{
- FILE *fp;
- char *buf, *lbuf, *file;
- size_t len, fid;
-
- logmsg(LOG_DEBUG, "[priv]: msg PRIV_GETLINES received");
-
- must_read(fd, , sizeof(size_t));
- if (fid >= NUM_FILETAB)
- errx(1, "invalid file id");
-
- file = file_table[fid].name;
-
- if (file == NULL)
- errx(1, "invalid file referenced");
-
- if (file_table[fid].count >= file_table[fid].max)
- errx(1, "maximum open count exceeded for %s", file);
-
- file_table[fid].count++;
-
- if ((fp = fopen(file, "r")) == NULL) {
- write_zero(fd);
- return;
- }
-
- lbuf = NULL;
- while ((buf = fgetln(fp, ))) {
- if (buf[len - 1] == '\n')
- buf[len - 1] = '\0';
- else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, NULL);
- memcpy(lbuf, buf, len);
- lbuf[len] = '\0';
- buf = lbuf;
- }
-
- write_string(fd, buf);
-
- free(lbuf);
- lbuf = NULL;
- }
- write_zero(fd);
- fclose(fp);
-}
-
-static void
tcpdump: revisiting some old diffs, hoist opening of pf.os.
I'm revisiting some old tcpdump diffs, now that mestre@ has added proper
unveil(2) support! :-)
Refresher: https://marc.info/?l=openbsd-tech=150535073209723=2
This hoists opening pf.os(5) fingerprints '-o' from the 'RUN' state to
the 'FILTER' state, this will allow for a reduced pledge(2) at runtime
in the (currently root) monitor process.
This still works as well as it already has. :-)
( ... ) [tcp sum ok] (src OS: OpenBSD 6.1) 3311509932:3311509932(0) win
16384
(DF) (ttl 64, id 41239, len 64)
The only potential difference is that if /etc/pf.os is replaced at
runtime, tcpdump won't reopen it.
I don't think that's a problem..
ok?
-Bryan.
Index: pfctl_osfp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/pfctl_osfp.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 pfctl_osfp.c
--- usr.sbin/tcpdump/pfctl_osfp.c 28 May 2017 10:06:12 - 1.13
+++ usr.sbin/tcpdump/pfctl_osfp.c 7 Nov 2018 23:52:48 -
@@ -81,17 +81,14 @@ void print_name_list(int, struct
name
voidsort_name_list(int, struct name_list *);
struct name_entry *lookup_name_list(struct name_list *, const char *);
-/* XXX arbitrary */
-#define MAX_FP_LINE 1024
-
/* Load fingerprints from a file */
int
pfctl_file_fingerprints(int dev, int opts, const char *fp_filename)
{
- u_char buf[MAX_FP_LINE];
+ FILE *in;
u_char *line;
size_t len;
- int i, lineno = 0;
+ int i, fd, lineno = 0;
int window, w_mod, ttl, df, psize, p_mod, mss, mss_mod, wscale,
wscale_mod, optcnt, ts0;
pf_tcpopts_t packed_tcpopts;
@@ -99,15 +96,22 @@ pfctl_file_fingerprints(int dev, int opt
struct pf_osfp_ioctl fp;
pfctl_flush_my_fingerprints();
+
+ fd = priv_open_pfosfp();
+ if (fd < 0)
+ return (1);
+
+ if ((in = fdopen(fd, "r")) == NULL) {
+ warn("%s", fp_filename);
+ return (1);
+ }
+
class = version = subtype = desc = tcpopts = NULL;
if ((opts & PF_OPT_NOACTION) == 0)
pfctl_clear_fingerprints(dev, opts);
- priv_getlines(FTAB_PFOSFP);
- while ((len = priv_getline(buf, sizeof(buf))) > 0) {
- buf[len -1] = '\n';
- line = buf;
+ while ((line = fgetln(in, )) != NULL) {
lineno++;
free(class);
free(version);
Index: privsep.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.49
diff -u -p -u -r1.49 privsep.c
--- usr.sbin/tcpdump/privsep.c 28 Sep 2018 06:48:59 - 1.49
+++ usr.sbin/tcpdump/privsep.c 7 Nov 2018 23:52:48 -
@@ -73,7 +73,8 @@ static const int allowed_max[] = {
/* INIT */ ALLOW(PRIV_OPEN_BPF) | ALLOW(PRIV_OPEN_DUMP) |
ALLOW(PRIV_SETFILTER),
/* BPF */ ALLOW(PRIV_SETFILTER),
- /* FILTER */ALLOW(PRIV_OPEN_OUTPUT) | ALLOW(PRIV_GETSERVENTRIES) |
+ /* FILTER */ALLOW(PRIV_OPEN_PFOSFP) | ALLOW(PRIV_OPEN_OUTPUT) |
+ ALLOW(PRIV_GETSERVENTRIES) |
ALLOW(PRIV_GETPROTOENTRIES) |
ALLOW(PRIV_ETHER_NTOHOST) | ALLOW(PRIV_INIT_DONE),
/* RUN */ ALLOW(PRIV_GETHOSTBYADDR) | ALLOW(PRIV_ETHER_NTOHOST) |
@@ -114,6 +115,7 @@ extern void set_slave_signals(void);
static voidimpl_open_bpf(int, int *);
static voidimpl_open_dump(int, const char *);
+static voidimpl_open_pfosfp(int);
static voidimpl_open_output(int, const char *);
static voidimpl_setfilter(int, char *, int *);
static voidimpl_init_done(int, int *);
@@ -277,6 +279,8 @@ priv_exec(int argc, char *argv[])
allowed_ext[STATE_RUN] |= ALLOW(PRIV_GETRPCBYNUMBER);
allowed_ext[STATE_FILTER] |= ALLOW(PRIV_GETPROTOENTRIES);
}
+ if (oflag)
+ allowed_ext[STATE_FILTER] |= ALLOW(PRIV_OPEN_PFOSFP);
if (infile)
cmdbuf = read_infile(infile);
@@ -297,6 +301,10 @@ priv_exec(int argc, char *argv[])
test_state(cmd, STATE_BPF);
impl_open_dump(sock, RFileName);
break;
+ case PRIV_OPEN_PFOSFP:
+ test_state(cmd, STATE_FILTER);
+ impl_open_pfosfp(sock);
+ break;
case PRIV_OPEN_OUTPUT:
test_state(cmd, STATE_RUN);
impl_open_output(sock, WFileName);
@@ -309,10 +317,6 @@ priv_exec(int argc, char *argv[])
test_state(cmd, STATE_RUN);
impl_init_done(sock, );
- if (oflag) {
- if (unveil("/etc/pf.os", "r") == -1)
- err(1,
Re: bypass support for iommu on sparc64
This is OpenBSD tech@ On Sat, Oct 20, 2018 at 08:36:33PM +0100, Andrew Grillet wrote: > So, substitute opening and closing the connection to the network? > > Is the IOMMU not used for disk (and all SCSI) access also? > > > > On Sat, 20 Oct 2018 at 20:32, Theo de Raadt wrote: > > > Andrew Grillet wrote: > > > > > Ok, what I am proposing is that the IOMMU is set up when a file is opened > > > to provide the address space required for that file's IO. > > > > Wow, you keep saying file as if it means something. > > > > packets off the network are not associated with any specific "file" > > activity > > > > it isn't how the kernel works. > > > > You are ... way off target. > > >
Re: pflogd unveil
On Thu, Aug 16, 2018 at 04:28:03PM -0400, Bryan Steele wrote:
> On Thu, Aug 16, 2018 at 04:20:54PM -0400, Bryan Steele wrote:
> > This adds unveil to pflogd(8)
> >
> > pflogd(8) is a special case, residing in /sbin, it's a static PIE. As
> > such, I thought it might be worth experimenting with execpromises here.
> > This allows re-exec after privdrop, and removes chroot(2) in favour of
> > only unveil(2) and pledge(2). I've left the code there for now just in
> > case portability is a concern.
>
> Please ignore this diff for now.
>
> > The privileged part of pflogd(8) is now disallowed from accessing most
> > of the filesystem, veiled except for read-only opening of /dev/bpf, and
> > rwc for the log file, typically /var/log/pflog. This process cannot yet
> > pledge(2), so special care is needed to make sure no library functions
> > called use files permitted by certain pledges.
> >
> > The unprivileged part already runs pledged "stdio recvfd"
> >
> > This includes the diff I sent previously, I'd like to commit that part
> > separately, any oks on that one?
> >
> > https://marc.info/?l=openbsd-tech=153347271628532=2
> >
> > Also, ok for this? ;-)
> >
> > -Bryan.
Sorry about that. New diff.
Index: pflogd.8
===
RCS file: /cvs/src/sbin/pflogd/pflogd.8,v
retrieving revision 1.49
diff -u -p -u -r1.49 pflogd.8
--- sbin/pflogd/pflogd.830 May 2017 17:15:06 - 1.49
+++ sbin/pflogd/pflogd.816 Aug 2018 20:36:11 -
@@ -86,9 +86,8 @@ temporarily uses the old snaplen to keep
tries to preserve the integrity of the log file against I/O errors.
Furthermore, integrity of an existing log file is verified before
appending.
-If there is an invalid log file or an I/O error, the log file is moved
-out of the way and a new one is created.
-If a new file cannot be created, logging is suspended until a
+If there is an invalid log file or an I/O error, logging is suspended
+until a
.Dv SIGHUP
or a
.Dv SIGALRM
Index: pflogd.c
===
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.58
diff -u -p -u -r1.58 pflogd.c
--- sbin/pflogd/pflogd.c9 Sep 2017 13:02:52 - 1.58
+++ sbin/pflogd/pflogd.c16 Aug 2018 20:36:11 -
@@ -75,7 +75,7 @@ int flush_buffer(FILE *);
int if_exists(char *);
void logmsg(int, const char *, ...);
void purge_buffer(void);
-int reset_dump(int);
+int reset_dump(void);
int scan_dump(FILE *, off_t);
int set_snaplen(int);
void set_suspended(int);
@@ -84,8 +84,6 @@ void sig_close(int);
void sig_hup(int);
void usage(void);
-static int try_reset_dump(int);
-
/* buffer must always be greater than snaplen */
static intbufpkt = 0; /* number of packets in buffer */
static intbuflen = 0; /* allocated size of buffer */
@@ -199,6 +197,11 @@ if_exists(char *ifname)
int
init_pcap(void)
{
+ if (unveil("/dev/bpf", "r") == -1) {
+ logmsg(LOG_ERR, "unveil: %s", strerror(errno));
+ return (-1);
+ }
+
hpcap = pcap_open_live(interface, snaplen, 1, PCAP_TO_MS, errbuf);
if (hpcap == NULL) {
logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
@@ -220,6 +223,11 @@ init_pcap(void)
return (-1);
}
+ if (unveil(NULL, NULL) == -1) {
+ logmsg(LOG_ERR, "unveil: %s", strerror(errno));
+ return (-1);
+ }
+
return (0);
}
@@ -238,25 +246,7 @@ set_snaplen(int snap)
}
int
-reset_dump(int nomove)
-{
- int ret;
-
- for (;;) {
- ret = try_reset_dump(nomove);
- if (ret <= 0)
- break;
- }
-
- return (ret);
-}
-
-/*
- * tries to (re)open log file, nomove flag is used with -x switch
- * returns 0: success, 1: retry (log moved), -1: error
- */
-int
-try_reset_dump(int nomove)
+reset_dump(void)
{
struct pcap_file_header hdr;
struct stat st;
@@ -323,12 +313,9 @@ try_reset_dump(int nomove)
}
} else if (scan_dump(fp, st.st_size)) {
fclose(fp);
- if (nomove || priv_move_log()) {
- logmsg(LOG_ERR,
- "Invalid/incompatible log file, move it away");
- return (-1);
- }
- return (1);
+ logmsg(LOG_ERR,
+ "Invalid/incompatible log file, move it away");
+ return (-1);
}
dpcap = fp;
@@ -641,7 +628,7 @@ main(int argc, char **argv)
bufpkt = 0;
}
- if (reset_dump(Xflag) < 0) {
+
