Re: ix(4): align rx payloads to the end of the cluster

2019-02-24 Thread Claudio Jeker
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

2019-02-24 Thread Matthias Schmidt

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

2019-02-24 Thread David Gwynne
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

2019-02-24 Thread David Gwynne
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

2019-02-24 Thread David Gwynne
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

2019-02-24 Thread David Gwynne



> 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

2019-02-24 Thread Ted Unangst
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)

2019-02-24 Thread Todd C . Miller
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

2019-02-24 Thread Matthias Schmidt
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.

2019-02-24 Thread Greg Steuck
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

2019-02-24 Thread Theo de Raadt
>> 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

2019-02-24 Thread Mark Kettenis
> 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)

2019-02-24 Thread Patrick Wildt
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

2019-02-24 Thread Landry Breuil
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

2019-02-24 Thread 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?


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

2019-02-24 Thread Hiltjo Posthuma
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

2019-02-24 Thread Matthieu Herrb
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

2019-02-24 Thread Stuart Henderson
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

2019-02-24 Thread Jeremie Courreges-Anglas
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)

2019-02-24 Thread Ingo Schwarze
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.