Re: ix(4): align rx payloads to the end of the cluster
On Mon, Feb 25, 2019 at 10:49:16AM +1000, David Gwynne wrote: > the mcl2k2 pool, aka the intel mbuf cluster pool, gets set up to allocate > at least 2048 + 2 bytes, which gets rounded up by 64 bytes to 2112 > bytes. this diff makes ix move the reception of packets to the end of > the 2112 byte allocation so there's space left at the front of the mbuf. > > this in turn makes it more likely that an m_prepend at another point in > the system will work without an extra mbuf allocation. eg, if you're > bridging or routing between vlans and vlans on svlans somewhere else, > this will be a bit faster with this diff. > > thoughts? ok? I think using m_align() here may be benefitial. Since it does exactly that. Apart from that I have to agree, shifting the packet back makes a lot of sense. > Index: dev/pci/if_ix.c > === > RCS file: /cvs/src/sys/dev/pci/if_ix.c,v > retrieving revision 1.152 > diff -u -p -r1.152 if_ix.c > --- dev/pci/if_ix.c 22 Jun 2017 02:44:37 - 1.152 > +++ dev/pci/if_ix.c 25 Feb 2019 00:40:47 - > @@ -2445,7 +2445,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i > return (ENOBUFS); > > mp->m_len = mp->m_pkthdr.len = sc->rx_mbuf_sz; > - m_adj(mp, ETHER_ALIGN); > + m_adj(mp, mp->m_ext.ext_size - sc->rx_mbuf_sz); > > error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->map, > mp, BUS_DMA_NOWAIT); > -- :wq Claudio
Re: Harmonize examples in signify man page
Hmm, somehow a leading space snuck in. Attached is the same diff without leading space and it now applies cleanly. Cheers Matthias On 24.02.2019 22:09, Matthias Schmidt wrote: Hi, while looking at signify's man page I noticed that the names of the arguments in the EXAMPLES section change (newkey.sec vs key.sec, etc). The diff harmonizes the examples. It makes it easier to follow the examples. Cheers diff --git signify.1 signify.1 index de3b433e3b0..9f7335352a2 100644 --- signify.1 +++ signify.1 @@ -153,13 +153,13 @@ The message file is too large. .El .Sh EXAMPLES Create a new key pair: -.Dl $ signify -G -p newkey.pub -s newkey.sec +.Dl $ signify -G -p key.pub -s key.sec .Pp Sign a file, specifying a signature name: -.Dl $ signify -S -s key.sec -m message.txt -x msg.sig +.Dl $ signify -S -s key.sec -m message -x message.sig .Pp Verify a signature, using the default signature name: -.Dl $ signify -V -p key.pub -m generalsorders.txt +.Dl $ signify -V -p key.pub -m message .Pp Verify a release directory containing .Pa SHA256.sig @@ -175,7 +175,7 @@ $ signify -C -p /etc/signify/openbsd-65-base.pub -x SHA256.sig bsd.rd .Pp Sign a gzip archive: .Bd -literal -offset indent -compact -$ signify -Sz -s key-arc.sec -m in.tgz -x out.tgz +$ signify -Sz -s key.sec -m in.tgz -x out.tgz .Ed .Pp Verify a gzip pipeline:
Re: ioctls for MPLS pseudowire interface config
On Mon, Feb 25, 2019 at 10:37:40AM +1000, David Gwynne wrote: > > > > On 22 Feb 2019, at 05:01, Martin Pieuchot wrote: > > > > On 21/02/19(Thu) 07:35, David Gwynne wrote: > >>> On 20 Feb 2019, at 11:21 pm, Martin Pieuchot wrote: > >>> > >>> On 20/02/19(Wed) 14:44, David Gwynne wrote: > Index: sys/net/if.c > === > RCS file: /cvs/src/sys/net/if.c,v > retrieving revision 1.571 > diff -u -p -r1.571 if.c > --- sys/net/if.c 9 Jan 2019 01:14:21 - 1.571 > +++ sys/net/if.c 20 Feb 2019 04:35:42 - > @@ -2143,6 +2143,25 @@ ifioctl(struct socket *so, u_long cmd, c > NET_UNLOCK(); > break; > > +case SIOCSETMPWCFG: > +case SIOCSPWE3CTRLWORD: > +case SIOCSPWE3FAT: > +case SIOCSPWE3NEIGHBOR: > +case SIOCDPWE3NEIGHBOR: > +if ((error = suser(p)) != 0) > +break; > +/* FALLTHROUGH */ > +case SIOCGETMPWCFG: > +case SIOCGPWE3CTRLWORD: > +case SIOCGPWE3FAT: > +case SIOCGPWE3NEIGHBOR: > +if_ref(ifp); > +KERNEL_UNLOCK(); > +error = ((*ifp->if_ioctl)(ifp, cmd, data)); > +KERNEL_LOCK(); > +if_put(ifp); > >>> > >>> Why are you referencing the `ifp' and grabbing the KERNEL_LOCK() > >>> (recursively)? > >> > >> ifioctl gets the ifp pointer from ifunit, which doesn't increase the ref > >> count for you. I'm giving up kernel lock around the pwe3 ioctl calls into > >> the driver, not taking them harder. Taking the ifp ref there guarantees > >> the interface will stay alive^Wallocated over those calls. > > > > It feels premature to me, well I'm confused. None of the other ioctl > > handlers do that. The KERNEL_LOCK() is still held in ifioctl() which > > guarantees serialization. If any of the ioctl(2) calls is going to sleep, > > thus releasing the lock, then I'd suggest to grab the NET_RLOCK() here > > instead. It still guarantees serialization of network ioctls w/ regard > > to detach. > > > > Note that I'll be delighted if you want to remove/push down the NET_LOCK() > > from this code path, but can we keep the handlers coherent? > > > > Even if we're using refcounting, don't we want to serialize all network > > ioctl(2)s? If we're not using the NET_LOCK() for this, can this new lock > > guarantee that that `ifp' isn't going away? Or do you have a better > > idea? > > The network stack implicitly taking locks is hurting me way more > than it's helping me at the moment, particularly the net lock, so > I would like to spend some time narrowing that down. If the consensus > is that it's too much risk for drivers to keep themselves consistent > then I'd argue for a per ifp rwlock that the ifioctl code could > take and release. > > Do you want me to do that for the pwe3 ioctls? There's a small > number of MPLS interfaces, so they'd be good for a test run. > > ifunit() is notionally like if_get except it doesn't give you a > reference. You have to be holding a lock that prevents the interface > being removed from the list if you're calling ifunit. The code > doesn't make it clear whether the lock you need to be holding is > the kernel lock or the net lock, but the kernel lock is empirically > good enough. If you give up that lock while holding the ifp, you > need to account for your reference to it. deraadt@ talked me down from giving up KERNEL_LOCK. so this is what the diff would be like if the interface had a lock and it was taken around the mpls ioctls. my opinion on the pros and cons of this is: pro: it keeps the individual driver state consistent cos changes are serialised by the lock. this means you don't have to think too hard about the driver locking against itself. pro: it allows fear free use of ifq_barrier. ifq_barrier cannot deadlock if the caller isn't holding NET_LOCK. this is the big win because it supports the model where the ioctl can coordinate with the running stack by publishing a change and then inserting a barrier to ensure the old state is no longer in use. without this the ioctl will have to give up the implicit NET_LOCK it has. con: only the mpls/pwe3 ioctls are covered. but i have to start somewhere, right? Index: if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.571 diff -u -p -r1.571 if.c --- if.c9 Jan 2019 01:14:21 - 1.571 +++ if.c25 Feb 2019 04:46:43 - @@ -594,6 +594,8 @@ if_attach_common(struct ifnet *ifp) { KASSERT(ifp->if_ioctl != NULL); + rw_init(&ifp->if_lock, ifp->if_xname); + TAILQ_INIT(&ifp->if_addrlist); TAILQ_INIT(&ifp->if_maddrlist); @@ -2141,6 +2143,27 @@ ifioctl(struct socket *so, u_long cmd, c
add the RETURN VALUES section to rwlock.9
i had to think a bit about what rw_lock returns, so i made this. is this worth it? Index: rwlock.9 === RCS file: /cvs/src/share/man/man9/rwlock.9,v retrieving revision 1.23 diff -u -p -r1.23 rwlock.9 --- rwlock.94 Jun 2018 04:52:33 - 1.23 +++ rwlock.925 Feb 2019 01:27:21 - @@ -188,19 +188,7 @@ functions check the status panicking if it is not write-, read-, any-, or unlocked, respectively. .Pp .Nm rw_status -returns the current state of the lock: -.Pp -.Bl -tag -width "RW_WRITE_OTHER" -offset indent -compact -.It Dv RW_WRITE -Lock is write locked by the calling thread. -.It Dv RW_WRITE_OTHER -Lock is write locked by a different thread. -.It Dv RW_READ -Lock is read locked. -The current thread may be one of the threads that has it locked. -.It 0 -Lock is not locked. -.El +returns the current state of the lock. .Pp A lock declaration may be initialised with the .Fn RWLOCK_INITIALIZER @@ -223,6 +211,30 @@ and can be called during autoconf, from process context, or from interrupt context. .Pp All other functions can be called during autoconf or from process context. +.Sh RETURN VALUES +.Nm rw_enter +and +.Nm rrw_enter +return 0 on success, or an +.Xr errno 2 +style value on failure. +.Pp +.Nm rw_status +and +.Nm rrw_status +return the state of the lock: +.Pp +.Bl -tag -width "RW_WRITE_OTHER" -offset indent -compact +.It Dv RW_WRITE +Lock is write locked by the calling thread. +.It Dv RW_WRITE_OTHER +Lock is write locked by a different thread. +.It Dv RW_READ +Lock is read locked. +The current thread may be one of the threads that has it locked. +.It 0 +Lock is not locked. +.El .Sh SEE ALSO .Xr witness 4 , .Xr mutex 9 ,
ix(4): align rx payloads to the end of the cluster
the mcl2k2 pool, aka the intel mbuf cluster pool, gets set up to allocate at least 2048 + 2 bytes, which gets rounded up by 64 bytes to 2112 bytes. this diff makes ix move the reception of packets to the end of the 2112 byte allocation so there's space left at the front of the mbuf. this in turn makes it more likely that an m_prepend at another point in the system will work without an extra mbuf allocation. eg, if you're bridging or routing between vlans and vlans on svlans somewhere else, this will be a bit faster with this diff. thoughts? ok? Index: dev/pci/if_ix.c === RCS file: /cvs/src/sys/dev/pci/if_ix.c,v retrieving revision 1.152 diff -u -p -r1.152 if_ix.c --- dev/pci/if_ix.c 22 Jun 2017 02:44:37 - 1.152 +++ dev/pci/if_ix.c 25 Feb 2019 00:40:47 - @@ -2445,7 +2445,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i return (ENOBUFS); mp->m_len = mp->m_pkthdr.len = sc->rx_mbuf_sz; - m_adj(mp, ETHER_ALIGN); + m_adj(mp, mp->m_ext.ext_size - sc->rx_mbuf_sz); error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->map, mp, BUS_DMA_NOWAIT);
Re: ioctls for MPLS pseudowire interface config
> On 22 Feb 2019, at 05:01, Martin Pieuchot wrote: > > On 21/02/19(Thu) 07:35, David Gwynne wrote: >>> On 20 Feb 2019, at 11:21 pm, Martin Pieuchot wrote: >>> >>> On 20/02/19(Wed) 14:44, David Gwynne wrote: Index: sys/net/if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.571 diff -u -p -r1.571 if.c --- sys/net/if.c 9 Jan 2019 01:14:21 - 1.571 +++ sys/net/if.c 20 Feb 2019 04:35:42 - @@ -2143,6 +2143,25 @@ ifioctl(struct socket *so, u_long cmd, c NET_UNLOCK(); break; + case SIOCSETMPWCFG: + case SIOCSPWE3CTRLWORD: + case SIOCSPWE3FAT: + case SIOCSPWE3NEIGHBOR: + case SIOCDPWE3NEIGHBOR: + if ((error = suser(p)) != 0) + break; + /* FALLTHROUGH */ + case SIOCGETMPWCFG: + case SIOCGPWE3CTRLWORD: + case SIOCGPWE3FAT: + case SIOCGPWE3NEIGHBOR: + if_ref(ifp); + KERNEL_UNLOCK(); + error = ((*ifp->if_ioctl)(ifp, cmd, data)); + KERNEL_LOCK(); + if_put(ifp); >>> >>> Why are you referencing the `ifp' and grabbing the KERNEL_LOCK() >>> (recursively)? >> >> ifioctl gets the ifp pointer from ifunit, which doesn't increase the ref >> count for you. I'm giving up kernel lock around the pwe3 ioctl calls into >> the driver, not taking them harder. Taking the ifp ref there guarantees the >> interface will stay alive^Wallocated over those calls. > > It feels premature to me, well I'm confused. None of the other ioctl > handlers do that. The KERNEL_LOCK() is still held in ifioctl() which > guarantees serialization. If any of the ioctl(2) calls is going to sleep, > thus releasing the lock, then I'd suggest to grab the NET_RLOCK() here > instead. It still guarantees serialization of network ioctls w/ regard > to detach. > > Note that I'll be delighted if you want to remove/push down the NET_LOCK() > from this code path, but can we keep the handlers coherent? > > Even if we're using refcounting, don't we want to serialize all network > ioctl(2)s? If we're not using the NET_LOCK() for this, can this new lock > guarantee that that `ifp' isn't going away? Or do you have a better > idea? The network stack implicitly taking locks is hurting me way more than it's helping me at the moment, particularly the net lock, so I would like to spend some time narrowing that down. If the consensus is that it's too much risk for drivers to keep themselves consistent then I'd argue for a per ifp rwlock that the ifioctl code could take and release. Do you want me to do that for the pwe3 ioctls? There's a small number of MPLS interfaces, so they'd be good for a test run. ifunit() is notionally like if_get except it doesn't give you a reference. You have to be holding a lock that prevents the interface being removed from the list if you're calling ifunit. The code doesn't make it clear whether the lock you need to be holding is the kernel lock or the net lock, but the kernel lock is empirically good enough. If you give up that lock while holding the ifp, you need to account for your reference to it.
Re: remove date from signify zsig
Stuart Henderson wrote: > On 2019/02/23 18:02, Ted Unangst wrote: > > signify -z adds a date= line to the header, but nothing reads it. It's also > > not very useful, since it's outside the signature. It would still not be > > useful, because nothing about the signify design cares about when something > > was signed. It does cause trouble, however, because signing the same thing > > twice results in two different files. Normal signify operation produces > > consistent signatures. > > pkg_add reads this header and copies to the @digital-signature line > in the +CONTENTS file. It is directly user visible too, for the "always > updated" quirks package, the @digital-signature line is read and displayed: I was trying to find such code, but obviously failed. > I'm not sure what you mean "outside the signature", changing the > date string does cause validation to fail, so it must be covered by > the signature? Ah, it is. Never mind then. The context is that some people are trying to use signify in a determinisitic reproducible way, and the dates keep changing. At first this looked like an unnecessary addition, but if we're using it, then that's how things are.
Re: start cleaning up UTF-8 processing in less(1)
On Sun, 24 Feb 2019 11:40:10 +0100, Ingo Schwarze wrote: > During the upcoming cleanup steps, let use retain full support for > the first (ESC-[) syntax and lets us completely delete support for > the second and third CSI syntaxes (single-byte CSI and UTF-8 > single-character two-byte CSI). That makes the most sense to me. The simpler we can make less(1) the better. - todd
Harmonize examples in signify man page
Hi, while looking at signify's man page I noticed that the names of the arguments in the EXAMPLES section change (newkey.sec vs key.sec, etc). The diff harmonizes the examples. It makes it easier to follow the examples. Cheers Matthias diff --git signify.1 signify.1 index de3b433e3b0..9f7335352a2 100644 --- signify.1 +++ signify.1 @@ -153,13 +153,13 @@ The message file is too large. .El .Sh EXAMPLES Create a new key pair: -.Dl $ signify -G -p newkey.pub -s newkey.sec +.Dl $ signify -G -p key.pub -s key.sec .Pp Sign a file, specifying a signature name: -.Dl $ signify -S -s key.sec -m message.txt -x msg.sig +.Dl $ signify -S -s key.sec -m message -x message.sig .Pp Verify a signature, using the default signature name: -.Dl $ signify -V -p key.pub -m generalsorders.txt +.Dl $ signify -V -p key.pub -m message .Pp Verify a release directory containing .Pa SHA256.sig @@ -175,7 +175,7 @@ $ signify -C -p /etc/signify/openbsd-65-base.pub -x SHA256.sig bsd.rd .Pp Sign a gzip archive: .Bd -literal -offset indent -compact -$ signify -Sz -s key-arc.sec -m in.tgz -x out.tgz +$ signify -Sz -s key.sec -m in.tgz -x out.tgz .Ed .Pp Verify a gzip pipeline:
In emacs: Use of uninitialized value in concatenation (.) or string at /usr/ports/infrastructure/lib/DPB/MiniCurses.pm line 279.
Hi Marc, I started dpb in emacs compilation mode (I know...) and observed these errors: Use of uninitialized value in concatenation (.) or string at /usr/ports/infrastructure/lib/DPB/MiniCurses.pm line 279. A little bit of digging revealed that $self->{home} is undefined, namely making this change @@ -276,7 +276,7 @@ sub write_home { my ($self, $msg) = @_; my @new = $self->cut_lines($msg); - print $self->{home}.$self->lines(@new); + print (($self->{home} || "?HOME?").$self->lines(@new)); $self->{oldlines} = \@new; } ... resulted in the output at the bottom. My perl too rusty to figure out how it gets there, but clearly emacs terminal lies aren't agreeable with MiniCurses.pm. To make things more interesting, I'm also running dpb via tramp with emacs on a different machine. The environment where dpb finds itself looks something like this: -*- mode: compilation; default-directory: "/sshx:obsd-build:/usr/ports/infrastructure/lib/DPB/" -*- doas env COLUMNS=88 ENV='' HISTFILE=/home/syzkaller/.tramp_history HOME=/home/syzkaller INSIDE_EMACS=26.1,compile LC_ALL=en_US.UTF-8 LC_CTYPE='' LOGNAME=syzkaller OLDPWD=/home/syzkaller PAGER=cat PATH=/usr/bin:/bin:/usr/sbin:/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin PROMPT_COMMAND= PS1=/sshx:obsd-build:/usr/ports/infrastructure/lib/DPB/ #$ PS2= PS3= PWD=/usr/ports/infrastructure/lib/DPB SHELL=/bin/ksh SSH_AUTH_SOCK=/tmp/ssh-ymKwcvVsaa/agent.45005 SSH_CLIENT=x.y.z.t 35760 22 SSH_CONNECTION=x.y.x.t 35760 a.b.c.d 22 SSH_TTY=/dev/ttyp4 TERM=dumb TERMCAP= TMOUT=0 USER=syzkaller _=/bin/sh Thanks Greg -*- mode: compilation; default-directory: "/sshx:obsd-build:/syzkaller/ports/" -*- Compilation started at Sun Feb 24 12:39:37 ulimit -d 33554432 ; doas /usr/ports/infrastructure/bin/dpb -P /syzkaller/hs-ports Started as: root Port user: syzkaller Build user: _pbuild Fetch user: _pfetch Log user: _pbuild Unpriv user: _dpb Reading build stats...zapping old stuff...Done zap duplicates...Done Waiting for hosts to finish STARTUP...ready on localhost 24 Feb 12:39:41 [56139] running for 00:00:03 LISTING [99861] at archivers/hs-zip-archive Hosts: localhost I=0 B=0 Q=0 T=0 F=0 !=0 24 Feb 12:39:41 [56139] running for 00:00:03 LISTING [99861] at audio/hs-libmpd http://goo.gl/6dMsr Fingerprint: 5E2B 2D0E 1E03 2046 BEC3 4D50 0B15 42BD 8DF5 A1B0
Re: Neuter shm calls from X swrast_dri.so
>> Date: Sun, 24 Feb 2019 10:44:25 -0500 >> From: Todd Mortimer >> >> A few weeks ago I noticed that firefox tabs were getting killed for >> running afoul of pledge(2). It seems that the problem was some calls to >> shmget(2) from the X swrast_dri.so lib that seem to have come from the >> new mesa code that was recently imported. Since the shm syscalls aren't >> covered by any pledge the system was killing the firefox tabs when they >> called into X and X went down this code path. >> >> The shm calls are guarded by a #ifdef, so the patch below just >> modifies the conditions so OpenBSD does not include the shm function and >> falls back to ordinary malloc. With this patch my firefox works again. >> >> The alternative is to add shm to pledge(2) somewhere. I expect that >> adding a syscall to pledge for a single program is unwanted, but in this >> case it would be for any program using X with this DRI module. A quick >> check in xenocara finds a small number of other references to the shm >> functions (lib/libXvMC/src/XvMC.c, lib/xcb-util-image/), but I don't >> know if we use these. >> >> ok? > >Sorry no. I don't think it makes sense to criple an important >optimization. Without shared memory support large bitmaps need to be >transferred across a socket with is a lot slower. > >Maybe we do need a SysV shared memory pledge. But there are >reasonable objections to including pledge support for an archaic >subsystem. > >These days X provides an alternative based on POSIX shared memory and >file descriptor passing. That might be a better match for a pledgable >interface. But the Mesa code doesn't support this yet. Also, it is always always reasonable disable pledge in a program which cannot fit to the pledge world view. pledge intentionally takes a hard line.
Re: Neuter shm calls from X swrast_dri.so
> Date: Sun, 24 Feb 2019 10:44:25 -0500 > From: Todd Mortimer > > A few weeks ago I noticed that firefox tabs were getting killed for > running afoul of pledge(2). It seems that the problem was some calls to > shmget(2) from the X swrast_dri.so lib that seem to have come from the > new mesa code that was recently imported. Since the shm syscalls aren't > covered by any pledge the system was killing the firefox tabs when they > called into X and X went down this code path. > > The shm calls are guarded by a #ifdef, so the patch below just > modifies the conditions so OpenBSD does not include the shm function and > falls back to ordinary malloc. With this patch my firefox works again. > > The alternative is to add shm to pledge(2) somewhere. I expect that > adding a syscall to pledge for a single program is unwanted, but in this > case it would be for any program using X with this DRI module. A quick > check in xenocara finds a small number of other references to the shm > functions (lib/libXvMC/src/XvMC.c, lib/xcb-util-image/), but I don't > know if we use these. > > ok? Sorry no. I don't think it makes sense to criple an important optimization. Without shared memory support large bitmaps need to be transferred across a socket with is a lot slower. Maybe we do need a SysV shared memory pledge. But there are reasonable objections to including pledge support for an archaic subsystem. These days X provides an alternative based on POSIX shared memory and file descriptor passing. That might be a better match for a pledgable interface. But the Mesa code doesn't support this yet. > Index: lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c > === > RCS file: /cvs/xenocara/lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c,v > retrieving revision 1.7 > diff -u -p -u -r1.7 dri_sw_winsys.c > --- lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c19 Feb 2019 > 04:24:01 - 1.7 > +++ lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c24 Feb 2019 > 15:16:35 - > @@ -26,8 +26,9 @@ > * > **/ > > -#if !defined(ANDROID) || ANDROID_API_LEVEL >= 26 > +#if (!defined(ANDROID) || ANDROID_API_LEVEL >= 26) && !defined(__OpenBSD__) > /* Android's libc began supporting shm in Oreo */ > +/* OpenBSD does not allow shm in programs using pledge(2) */ > #define HAVE_SHM > #include > #include > > >
Re: diff: add support for ANT-USBStick2 to uscom(4)
On Fri, Feb 22, 2019 at 08:49:58PM +0100, Jan Klemkow wrote: > Hi, > > The diff below adds support for the Dynastream "ANT USBStick2" to > uscom(4). The device attached with the following message: > > uscom0 at uhub0 port 2 configuration 1 interface 0 "Dynastream Innovations > ANT USBStick2" rev 2.00/1.00 addr 2 > ucom0 at uscom0 portno 0 > > Additionally, I tested the device with the command: > > # cu -l /dev/cuaU0 | od -h > ... a4 01 ae 00 0b ... > > It returns a valid ANT binary error message. > > Bye, > Jan Thanks, committed! I also made the manpage change to uscom(4) which lists the supported devices. > Index: dev/usb/usbdevs > === > RCS file: /cvs/src/sys/dev/usb/usbdevs,v > retrieving revision 1.694 > diff -u -p -r1.694 usbdevs > --- dev/usb/usbdevs 14 Jan 2019 03:28:03 - 1.694 > +++ dev/usb/usbdevs 22 Feb 2019 17:54:59 - > @@ -1640,6 +1640,7 @@ product DVICO RT30700xb307 RT3070 > product DYNASTREAM ANTDEVBOARD 0x1003 ANT dev board > product DYNASTREAM ANT2USB 0x1004 ANT2USB > product DYNASTREAM ANTDEVBOARD2 0x1006 ANT dev board > +product DYNASTREAM ANTUSB2 0x1008 ANTUSB-2 Stick > product DYNASTREAM ANTUSBM 0x1009 ANTUSB-m Stick > > /* EasyDisk products */ > Index: dev/usb/usbdevs.h > === > RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v > retrieving revision 1.706 > diff -u -p -r1.706 usbdevs.h > --- dev/usb/usbdevs.h 14 Jan 2019 03:28:51 - 1.706 > +++ dev/usb/usbdevs.h 22 Feb 2019 18:05:14 - > @@ -1,4 +1,4 @@ > -/* $OpenBSD: usbdevs.h,v 1.706 2019/01/14 03:28:51 jmatthew Exp $ */ > +/* $OpenBSD$ */ > > /* > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT. > @@ -1647,6 +1647,7 @@ > #define USB_PRODUCT_DYNASTREAM_ANTDEVBOARD 0x1003 /* ANT > dev board */ > #define USB_PRODUCT_DYNASTREAM_ANT2USB 0x1004 /* ANT2USB */ > #define USB_PRODUCT_DYNASTREAM_ANTDEVBOARD2 0x1006 /* ANT > dev board */ > +#define USB_PRODUCT_DYNASTREAM_ANTUSB2 0x1008 /* ANTUSB-2 > Stick */ > #define USB_PRODUCT_DYNASTREAM_ANTUSBM 0x1009 /* ANTUSB-m > Stick */ > > /* EasyDisk products */ > Index: dev/usb/usbdevs_data.h > === > RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v > retrieving revision 1.700 > diff -u -p -r1.700 usbdevs_data.h > --- dev/usb/usbdevs_data.h14 Jan 2019 03:28:51 - 1.700 > +++ dev/usb/usbdevs_data.h22 Feb 2019 18:05:14 - > @@ -1,4 +1,4 @@ > -/* $OpenBSD: usbdevs_data.h,v 1.700 2019/01/14 03:28:51 jmatthew Exp $ > */ > +/* $OpenBSD$ */ > > /* > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT. > @@ -2876,6 +2876,10 @@ const struct usb_known_product usb_known > { > USB_VENDOR_DYNASTREAM, USB_PRODUCT_DYNASTREAM_ANTDEVBOARD2, > "ANT dev board", > + }, > + { > + USB_VENDOR_DYNASTREAM, USB_PRODUCT_DYNASTREAM_ANTUSB2, > + "ANTUSB-2 Stick", > }, > { > USB_VENDOR_DYNASTREAM, USB_PRODUCT_DYNASTREAM_ANTUSBM, > Index: dev/usb/uscom.c > === > RCS file: /cvs/src/sys/dev/usb/uscom.c,v > retrieving revision 1.6 > diff -u -p -r1.6 uscom.c > --- dev/usb/uscom.c 22 Aug 2018 15:32:49 - 1.6 > +++ dev/usb/uscom.c 22 Feb 2019 17:55:44 - > @@ -53,6 +53,7 @@ struct ucom_methods uscom_methods = { > > static const struct usb_devno uscom_devs[] = { > { USB_VENDOR_HP,USB_PRODUCT_HP_HPX9GP }, > + { USB_VENDOR_DYNASTREAM,USB_PRODUCT_DYNASTREAM_ANTUSB2 }, > { USB_VENDOR_DYNASTREAM,USB_PRODUCT_DYNASTREAM_ANTUSBM } > }; > >
Re: Neuter shm calls from X swrast_dri.so
On Sun, Feb 24, 2019 at 10:44:25AM -0500, Todd Mortimer wrote: > A few weeks ago I noticed that firefox tabs were getting killed for > running afoul of pledge(2). It seems that the problem was some calls to > shmget(2) from the X swrast_dri.so lib that seem to have come from the > new mesa code that was recently imported. Since the shm syscalls aren't > covered by any pledge the system was killing the firefox tabs when they > called into X and X went down this code path. > > The shm calls are guarded by a #ifdef, so the patch below just > modifies the conditions so OpenBSD does not include the shm function and > falls back to ordinary malloc. With this patch my firefox works again. > > The alternative is to add shm to pledge(2) somewhere. I expect that > adding a syscall to pledge for a single program is unwanted, but in this > case it would be for any program using X with this DRI module. A quick > check in xenocara finds a small number of other references to the shm > functions (lib/libXvMC/src/XvMC.c, lib/xcb-util-image/), but I don't > know if we use these. Thanks for looking into this, and nice findings ! There has been some discussion to add an 'shm' pledge class, but no real usage surfaced so far, and the usual stance was to neuter the shmget calls so that fallback codepaths were used like in https://bugzilla.mozilla.org/show_bug.cgi?id=1457092. - maybe there are more in other programs.. but there would have been a lot of pledge reports if so.
Neuter shm calls from X swrast_dri.so
A few weeks ago I noticed that firefox tabs were getting killed for running afoul of pledge(2). It seems that the problem was some calls to shmget(2) from the X swrast_dri.so lib that seem to have come from the new mesa code that was recently imported. Since the shm syscalls aren't covered by any pledge the system was killing the firefox tabs when they called into X and X went down this code path. The shm calls are guarded by a #ifdef, so the patch below just modifies the conditions so OpenBSD does not include the shm function and falls back to ordinary malloc. With this patch my firefox works again. The alternative is to add shm to pledge(2) somewhere. I expect that adding a syscall to pledge for a single program is unwanted, but in this case it would be for any program using X with this DRI module. A quick check in xenocara finds a small number of other references to the shm functions (lib/libXvMC/src/XvMC.c, lib/xcb-util-image/), but I don't know if we use these. ok? Index: lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c === RCS file: /cvs/xenocara/lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c,v retrieving revision 1.7 diff -u -p -u -r1.7 dri_sw_winsys.c --- lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c 19 Feb 2019 04:24:01 - 1.7 +++ lib/mesa/src/gallium/winsys/sw/dri/dri_sw_winsys.c 24 Feb 2019 15:16:35 - @@ -26,8 +26,9 @@ * **/ -#if !defined(ANDROID) || ANDROID_API_LEVEL >= 26 +#if (!defined(ANDROID) || ANDROID_API_LEVEL >= 26) && !defined(__OpenBSD__) /* Android's libc began supporting shm in Oreo */ +/* OpenBSD does not allow shm in programs using pledge(2) */ #define HAVE_SHM #include #include
[patch] improve strptime(3) %z timezone parsing
Hi, I noticed some things in the strptime(3) implementing when parsing timezone strings using the %z format string. 1. I noticed the tm_gmtoff value is not correctly set in some cases. It should set tm_gmtoff to the offset from UTC in seconds (as described in mktime(3)). 2. The military/nautical UTC offsets are also reversed. This was also actually a bug in RFC822: RFC5322 (referenced in strptime(3) man page): https://tools.ietf.org/html/rfc5322 Section 4.3, page 34 says: " The 1 character military time zones were defined in a non-standard way in [RFC0822] and are therefore unpredictable in their meaning. The original definitions of the military zones "A" through "I" are equivalent to "+0100" through "+0900", respectively; "K", "L", and "M" are equivalent to "+1000", "+1100", and "+1200", respectively; "N" through "Y" are equivalent to "-0100" through "-1200". respectively; and "Z" is equivalent to "+". However, because of the error in [RFC0822], they SHOULD all be considered equivalent to "-" unless there is out-of-band information confirming their meaning. " While comparing the other BSD and Linux code-bases I noticed NetBSD has fixed this on 2017-08-24. It is not in NetBSD 8 stable yet, but in NetBSD-current: http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/time/strptime.c 3. The military zone J is also ambiguous. Some standards define it as unused, while others define it as "local observed time". NetBSD handles it as local observed time, but OpenBSD returns NULL in strptime(3). I left this as is. 4. While at it I also fixed some trailing whitespaces. Thanks for not falling asleep and reading the long text :) Patch and test program below: diff --git lib/libc/time/strptime.c lib/libc/time/strptime.c index eaf182dc773..86a0d5353ee 100644 --- lib/libc/time/strptime.c +++ lib/libc/time/strptime.c @@ -116,7 +116,7 @@ _strptime(const char *buf, const char *fmt, struct tm *tm, int initialize) fmt++; continue; } - + if ((c = *fmt++) != '%') goto literal; @@ -142,7 +142,7 @@ literal: _LEGAL_ALT(0); alt_format |= _ALT_O; goto again; - + /* * "Complex" conversion rules, implemented through recursion. */ @@ -380,7 +380,7 @@ literal: * number but without the century. */ if (!(_conv_num(&bp, &i, 0, 99))) - return (NULL); + return (NULL); continue; case 'G': /* The year corresponding to the ISO week @@ -500,7 +500,7 @@ literal: ep = _find_string(bp, &i, nast, NULL, 4); if (ep != NULL) { #ifdef TM_GMTOFF - tm->TM_GMTOFF = -5 - i; + tm->TM_GMTOFF = (-5 - i) * SECSPERHOUR; #endif #ifdef TM_ZONE tm->TM_ZONE = (char *)nast[i]; @@ -512,7 +512,7 @@ literal: if (ep != NULL) { tm->tm_isdst = 1; #ifdef TM_GMTOFF - tm->TM_GMTOFF = -4 - i; + tm->TM_GMTOFF = (-4 - i) * SECSPERHOUR; #endif #ifdef TM_ZONE tm->TM_ZONE = (char *)nadt[i]; @@ -527,11 +527,12 @@ literal: /* Argh! No 'J'! */ if (*bp >= 'A' && *bp <= 'I') tm->TM_GMTOFF = - ('A' - 1) - (int)*bp; + (int)*bp - ('A' - 1); else if (*bp >= 'L' && *bp <= 'M') - tm->TM_GMTOFF = 'A' - (int)*bp; + tm->TM_GMTOFF = (int)*bp - 'A'; else if (*bp >= 'N' && *bp <= 'Y') - tm->TM_GMTOFF = (int)*bp - 'M'; + tm->TM_GMTOFF = 'M' - (int)*bp; + tm->TM_GMTOFF *= SECSPERHOUR; #endif #ifdef TM_ZONE tm->TM_ZONE = NULL; /* XXX */ @@ -556,14 +557,15 @@ literal: } switch (i) { case 2: - offs *= 100; + offs *= SECSPERHOUR; break; case 4:
Re: switch Xorg protos to xorgproto 2018.4
On Thu, Feb 14, 2019 at 09:38:33AM +0100, Matthieu Herrb wrote: > On Mon, Feb 11, 2019 at 07:34:46AM +0100, Matthieu Herrb wrote: > > Hi, > > > > I've recently imported xorgproto 2018.4 in xenocara. This is a package > > the unifies all the previous *proto packages from X.Org (except > > xcb-proto which is special), and enabled it yesterday. > > > > I had to revert that last commit since it is suspected to be the cause > > for this regression mentionned on misc: > > https://marc.info/?l=openbsd-misc&m=154983711329128&w=2 > > > > So please test the following diff for other possible regressions, > > while we're looking at the compton issue. > > The compton issue has been found and a patch got committed to ports. > > So any ok for this ? ping ? > > > > > Index: Makefile > > === > > RCS file: /cvs/xenocara/proto/Makefile,v > > retrieving revision 1.17 > > diff -u -r1.17 Makefile > > --- Makefile10 Feb 2019 23:07:47 - 1.17 > > +++ Makefile11 Feb 2019 00:05:49 - > > @@ -2,17 +2,7 @@ > > > > .include > > > > -SUBDIR= bigreqsproto compositeproto dmxproto damageproto \ > > - fixesproto fontsproto glproto inputproto \ > > - kbproto pmproto xineramaproto presentproto randrproto \ > > - recordproto renderproto resourceproto scrnsaverproto \ > > - videoproto x11proto xcb-proto xcmiscproto xextproto \ > > - xf86bigfontproto xf86dgaproto \ > > - xf86vidmodeproto > > - > > -.if ${XENOCARA_BUILD_DRI:L} == "yes" > > -SUBDIR+= xf86driproto dri2proto dri3proto > > -.endif > > +SUBDIR= xcb-proto xorgproto > > > > .include > > > > -- > > Matthieu Herrb > > -- > Matthieu Herrb -- Matthieu Herrb
Re: remove date from signify zsig
On 2019/02/23 18:02, Ted Unangst wrote: > signify -z adds a date= line to the header, but nothing reads it. It's also > not very useful, since it's outside the signature. It would still not be > useful, because nothing about the signify design cares about when something > was signed. It does cause trouble, however, because signing the same thing > twice results in two different files. Normal signify operation produces > consistent signatures. pkg_add reads this header and copies to the @digital-signature line in the +CONTENTS file. It is directly user visible too, for the "always updated" quirks package, the @digital-signature line is read and displayed: # pkg_add -u quirks quirks-3.104 signed on 2019-02-23T23:46:16Z And at least some users make use of this to know when the package build was done. I'm not sure what you mean "outside the signature", changing the date string does cause validation to fail, so it must be covered by the signature?
Re: ld patch that greatly speeds up linking large programs with debug symbols
On Sat, Feb 23 2019, Aaron Miller wrote: > On February 23, 2019 2:50:46 AM PST, Jeremie Courreges-Anglas > wrote: >>On Sat, May 07 2016, Stefan Kempf wrote: [...] > Hi Jeremie, > > That is concerning. I'm on my phone and haven't had a chance to investigate, > but from the code in the gdb output above, it looks like the author of the > diff forgot to set the pointer to NULL after freeing. For example: > if (elf_tdata (sub)->symbuf) { > free (elf_tdata (sub)->symbuf); > elf_tdata (sub)->symbuf = NULL; > } > > This is not tested at all. I hope this works! It doesn't, which is consistent with the error seen with MALLOC_OPTIONS=S: "free (ptr=0xdbdbdbdbdbdbdbdb)" points out that the code uses uninitialized memory (0xdb). The 0xdf pattern in the sparc64 build failure is likely newly allocated, uninitialized memory which had previously been junked by free(3). -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: start cleaning up UTF-8 processing in less(1)
Hi, Stefan Sperling wrote on Sat, Feb 23, 2019 at 04:19:02PM +0100: > Your diff looks good to me. > And I can't see how it could make this situation any worse either. thanks for checking; i committed the first patch. To be able to continue with the less cleanup i started, i have to explain some theory first, and i'm kindly asking for advice which functionality is desired. Personally, i'd like to make less simpler and safer, removing some anachronistic, dangerous functionality, see below. Considerable complication in the less code results from support for https://en.wikipedia.org/wiki/ANSI_escape_code sequences. Note that i will *not* propose to delete that support completely, but only some more arcane and more dangerous parts of it, see at the very end of this mail. There are three ways to write an ANSI escape sequence; i'll have to explain all three in turn. First syntax: ESCAPE-BRACKET $ printf "normal \x1b[43m yellow \x1b[0m normal\n" Each ANSI escape sequence is introduced by the two-byte sequence consisting of the ASCII escape character followed by an ASCII opening square bracket. This form is relatively benign because it is both valid ASCII and valid UTF-8 and looks identical in both encodings. In our default configuration of xterm(1), which is moderately secure without being paranoid, the above command works out of the box, showing you the word "yellow" on a yellow background. It doesn't matter what LC_CTYPE is, it always works. By default, less(1) does *not* interpret such escape sequences, but escapes them and shows them as follows: normal ESC[43m yellow ESC[0m normal where the two instances of the string "ESC" are highlighted in reverse video. But if you run less(1) with the -R option, it passes the escape sequences on to the terminal, and again you see the word "yellow" on a yellow background, inside less(1). Horizontal scrolling works correctly, preserving vertical alignment of display columns, with the two escape sequences taking up zero columns each in an xterm(1). Your mileage may vary on exotic terminals, though. I'm conivinced this behaviour ought to be preserved. Second syntax: conventional 8-bit CSI - $ LC_CTYPE=C printf "normal \23343m yellow \2330m normal\n" Here, each ANSI escape sequence is introduced by the single byte 0x9b (CSI = control sequence introducer) instead of by the two-byte sequence ESC-[. Obviously, this implies that the second syntax is incompatible with UTF-8, so we always have to make sure we have LC_CTYPE=C set when doing anything with the second syntax. Note that i had to give the CSI bytes in octal in the printf command above; \x9b43m would not work because printf(1) would be unable to figure out where the hexadecimal character number ends. In our default xterm(1) configuration, this syntax is *NOT* supported. The above command prints the same as $ printf "normal \xef\xbf\27543m yellow \xef\xbf\2750m normal\n" where \xef\xbf\275 is the Unicode replacement character U+FFFD substituted for the (unsafe, ill-encoded) CSI. I you really want to, you can use the second syntax on OpenBSD, but that is *ABSOLUTELY NOT RECOMMENDED*. You would have to run xterm(1) in the unsafe, legacy, so called "conventional 8bit" mode as follows: $ LC_CTYPE=C xterm +lc # This is a really BAD idea. In such an xterm(1), the line $ LC_CTYPE=C printf "normal \23343m yellow \2330m normal\n" does show the word "yellow" in front of a yellow background. Reading the less(1) source code, i guess that it *intends* to support this, but it doesn't actually work for me: $ export LC_CTYPE=C $ printf "normal \23343m yellow \2330m normal\n" | less -R results in normal <9B>43m yellow <9B>0m normal for me, with the two strings "<9B>" highlighted in reverse video. Only with $ printf "normal \23343m yellow \2330m normal\n" | less -r do i see yellow background - but that is utterly useless because -r completely breaks column counting and wrapping in less(1), and besides it is irrelevent because -r doesn't need to inspect anything but simply passes *everything* through no matter what. I think it is better to completely remove support for the second syntax from less than to repair it. It is rarely used because it is incompatible with Unicode, it provides no advantages compared to the first syntax, but it painfully complicates the code. Third syntax: UTF-8 CSI --- $ printf "normal \xc2\23343m yellow \xc2\2330m normal\n" Here, each ANSI escape sequence is introduced by the Unicode character U+009B, represented in its two-byte UTF-8 encoding \xc2\x9b instead of by the two-byte sequence ESC-[. Obviously, this only mskes sense with a UTF-8 locale. However, the xterm documentation says in the file /usr/xenocara/app/xterm/ctlseqs.ms : It is not possible to use a C1 control obtained from decoding the UTF-8 text, because that would require reprocessing the data.