[PATCH 2/3] wireguard: increase if_txmit to 64

2020-06-22 Thread Jason A. Donenfeld
This increases throughput by keeping the worker threads active for
longer.
---
 sys/net/if_wg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git sys/net/if_wg.c sys/net/if_wg.c
index e27a575cc1b..5b7550840f8 100644
--- sys/net/if_wg.c
+++ sys/net/if_wg.c
@@ -2651,6 +2651,7 @@ wg_clone_create(struct if_clone *ifc, int unit)
ifp->if_mtu = DEFAULT_MTU;
ifp->if_flags = IFF_BROADCAST | IFF_MULTICAST | IFF_NOARP;
ifp->if_xflags = IFXF_CLONED;
+   ifp->if_txmit = 64; /* Keep our workers active for longer. */
 
ifp->if_ioctl = wg_ioctl;
ifp->if_start = wg_start;
-- 
2.27.0



[PATCH 1/3] conf: build wg(4) with ordinary kernel config

2020-06-22 Thread Jason A. Donenfeld
This will help us get a bit of testing in the snapshot builds.
---
 sys/conf/GENERIC | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git sys/conf/GENERIC sys/conf/GENERIC
index 34d608cff44..f0eff16d2b8 100644
--- sys/conf/GENERIC
+++ sys/conf/GENERIC
@@ -109,7 +109,7 @@ pseudo-device   vether  # Virtual ethernet
 pseudo-device  vxlan   # Virtual extensible LAN
 pseudo-device  vlan# IEEE 802.1Q VLAN
 pseudo-device  switch  # Switch
-#pseudo-device wg  # WireGuard
+pseudo-device  wg  # WireGuard
 
 pseudo-device  bio 1   # ioctl multiplexing device
 
-- 
2.27.0



[PATCH 3/3] wireguard: mark interface as MPSAFE

2020-06-22 Thread Jason A. Donenfeld
This enables us to process queueing of different packet queues without
KERNEL_LOCK. Combined with the increase in txlen, this keeps our
encryption workers more active. This results in a ~30% performance
boost.
---
 sys/net/if_wg.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git sys/net/if_wg.c sys/net/if_wg.c
index 5b7550840f8..25f79234d7b 100644
--- sys/net/if_wg.c
+++ sys/net/if_wg.c
@@ -1832,8 +1832,8 @@ wg_queue_out(struct wg_softc *sc, struct wg_peer *peer)
 
/*
 * We delist all staged packets and then add them to the queues. This
-* can race with wg_start when called from wg_send_keepalive, however
-* wg_start will not race as it is serialised.
+* can race with wg_qstart when called from wg_send_keepalive, however
+* wg_qstart will not race as it is serialised.
 */
mq_delist(>p_stage_queue, );
ml_init(_free);
@@ -2067,8 +2067,9 @@ wg_input(void *_sc, struct mbuf *m, struct ip *ip, struct 
ip6_hdr *ip6,
 }
 
 void
-wg_start(struct ifnet *ifp)
+wg_qstart(struct ifqueue *ifq)
 {
+   struct ifnet*ifp = ifq->ifq_if;
struct wg_softc *sc = ifp->if_softc;
struct wg_peer  *peer;
struct wg_tag   *t;
@@ -2079,11 +2080,10 @@ wg_start(struct ifnet *ifp)
 
/*
 * We should be OK to modify p_start_list, p_start_onlist in this
-* function as the interface is not IFXF_MPSAFE and therefore should
-* only be one instance of this function running at a time. These
-* values are not modified anywhere else.
+* function as there should only be one ifp->if_qstart invoked at a
+* time.
 */
-   while ((m = ifq_dequeue(>if_snd)) != NULL) {
+   while ((m = ifq_dequeue(ifq)) != NULL) {
t = wg_tag_get(m);
peer = t->t_peer;
if (mq_push(>p_stage_queue, m) != 0)
@@ -2163,7 +2163,7 @@ wg_output(struct ifnet *ifp, struct mbuf *m, struct 
sockaddr *sa,
 * delayed packet without doing some refcnting. If a peer is removed
 * while a delayed holds a reference, bad things will happen. For the
 * time being, delayed packets are unsupported. This may be fixed with
-* another aip_lookup in wg_start, or refcnting as mentioned before.
+* another aip_lookup in wg_qstart, or refcnting as mentioned before.
 */
if (m->m_pkthdr.pf.delay > 0) {
DPRINTF(sc, "PF Delay Unsupported");
@@ -2178,7 +2178,7 @@ wg_output(struct ifnet *ifp, struct mbuf *m, struct 
sockaddr *sa,
 
/*
 * We still have an issue with ifq that will count a packet that gets
-* dropped in wg_start, or not encrypted. These get counted as
+* dropped in wg_qstart, or not encrypted. These get counted as
 * ofails or oqdrops, so the packet gets counted twice.
 */
return if_enqueue(ifp, m);
@@ -2650,11 +2650,11 @@ wg_clone_create(struct if_clone *ifc, int unit)
 
ifp->if_mtu = DEFAULT_MTU;
ifp->if_flags = IFF_BROADCAST | IFF_MULTICAST | IFF_NOARP;
-   ifp->if_xflags = IFXF_CLONED;
+   ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
ifp->if_txmit = 64; /* Keep our workers active for longer. */
 
ifp->if_ioctl = wg_ioctl;
-   ifp->if_start = wg_start;
+   ifp->if_qstart = wg_qstart;
ifp->if_output = wg_output;
 
ifp->if_type = IFT_WIREGUARD;
-- 
2.27.0



make btrace interval event to reciprocal of ticks

2020-06-22 Thread Yuichiro NAITO
Current btrace has `interval:hz:1` probe that makes periodically events.
`interval:hz:1` looks to make events once per second (= 1Hz),
but current implementation makes once per tick (= 100Hz on amd64).
I think the interval should be counted by reciprocal of ticks so that fit to Hz.

Following patch changes to calculate 'dp_maxtick' fit to Hz.
'dtrq_rate' has number of hz. I think it should be allowed same value of 'hz'
so that we can use `interval:hz:100` that equals to once per tick on amd64.

Index: dt_prov_profile.c
===
RCS file: /cvs/src/sys/dev/dt/dt_prov_profile.c,v
retrieving revision 1.2
diff -u -p -r1.2 dt_prov_profile.c
--- dt_prov_profile.c   25 Mar 2020 14:59:23 -  1.2
+++ dt_prov_profile.c   23 Jun 2020 02:02:27 -
@@ -75,7 +75,7 @@ dt_prov_profile_alloc(struct dt_probe *d
KASSERT(TAILQ_EMPTY(plist));
KASSERT(dtp == dtpp_profile || dtp == dtpp_interval);
 
-   if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate >= hz)
+   if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate > hz)
return EOPNOTSUPP;
 
CPU_INFO_FOREACH(cii, ci) {
@@ -88,7 +88,7 @@ dt_prov_profile_alloc(struct dt_probe *d
return ENOMEM;
}
 
-   dp->dp_maxtick = dtrq->dtrq_rate;
+   dp->dp_maxtick = hz / dtrq->dtrq_rate;
dp->dp_cpuid = ci->ci_cpuid;
 
dp->dp_filter = dtrq->dtrq_filter;

-- 
Yuichiro NAITO
  naito.yuich...@gmail.com



Re: systat.1: Trim ":" description, support line kill character

2020-06-22 Thread Theo de Raadt
In OpenBSD, the erase character is ^?

^H is accepted in a few places, like here (because of CTRL_H) but
it is absolutely not the canonical tty 'character erase' character,
which is implied in your text by placing it next to kill and the
canonical tty werase default ^U

It was vague, and I guess OK.  But your increasing precision is making
it wrong.

Klemens Nanni  wrote:

> On Mon, Jun 22, 2020 at 06:33:30PM -0600, Theo de Raadt wrote:
> > > +character erase (^H) and line kill (^U) characters
> > 
> > ^H is wrong
> How so?  It is currently hardcoded as such in engine.c:cmd_keyboard():
> 
> 1188 switch (ch) {
> 1189 case KEY_ENTER:
> 1190 case 0x0a:
> 1191 case 0x0d:
> 1192 {
> 1193 struct command * c = command_set(NULL, NULL);
> 1194 c->exec(cmdbuf);
> 1195 break;
> 1196 }
> 1197 case KEY_BACKSPACE:
> 1198 case KEY_DC:
> 1199 case CTRL_H:
> 1200 if (cmd_len > 0) {
> 1201 cmdbuf[--cmd_len] = 0;
> 1202 } else
> 1203 beep();
> 1204 break;
> 1205 case 0x1b:
> 1206 case CTRL_G:
> 1207 if (cmd_len > 0) {
> 1208 cmdbuf[0] = '\0';
> 1209 cmd_len = 0;
> 1210 } else
> 1211 command_set(NULL, NULL);
> 1212 break;
> 1213 default:
> 1214 break;
> 1215 }
> 1216 }
> 



Re: systat.1: Trim ":" description, support line kill character

2020-06-22 Thread Klemens Nanni
On Mon, Jun 22, 2020 at 06:33:30PM -0600, Theo de Raadt wrote:
> > +character erase (^H) and line kill (^U) characters
> 
> ^H is wrong
How so?  It is currently hardcoded as such in engine.c:cmd_keyboard():

1188 switch (ch) {
1189 case KEY_ENTER:
1190 case 0x0a:
1191 case 0x0d:
1192 {
1193 struct command * c = command_set(NULL, NULL);
1194 c->exec(cmdbuf);
1195 break;
1196 }
1197 case KEY_BACKSPACE:
1198 case KEY_DC:
1199 case CTRL_H:
1200 if (cmd_len > 0) {
1201 cmdbuf[--cmd_len] = 0;
1202 } else
1203 beep();
1204 break;
1205 case 0x1b:
1206 case CTRL_G:
1207 if (cmd_len > 0) {
1208 cmdbuf[0] = '\0';
1209 cmd_len = 0;
1210 } else
1211 command_set(NULL, NULL);
1212 break;
1213 default:
1214 break;
1215 }
1216 }



Re: systat.1: Trim ":" description, support line kill character

2020-06-22 Thread Theo de Raadt
> +character erase (^H) and line kill (^U) characters

^H is wrong



Re: systat.1: Trim ":" description, support line kill character

2020-06-22 Thread Klemens Nanni
On Mon, Jun 22, 2020 at 07:13:30AM +0100, Jason McIntyre wrote:
> how will people be able to find this if we don;t document it? from a
> brief skim of docs which may hold answers, i still can;t find where
> these values are documented.
Fair point, I removed them because they imply that systat honours
whatever keys control character are bound to, even though it in fact
hardcodes keys such as ^H for character kill and ^G for line kill.

top(1)'s INTERACTIVE MODE section for example ends with

While typing this information in, the user's erase and kill
keys (as set up by the command stty(1)) are recognized, and a newline
terminates the input.

and this indeed correct for top, but systat(1) behaves differently as
can be easily observed when for example binding character kill to @
instead of ^?:

$ stty erase @
$ top
$ systat

Then use any interactive menu and note how in top @ now erases the
last character while backspace prints "^?", whereas in systat @ still
prints "@" and backspace still erase the last character.

> couldn;t we document these work and what the defaults are? it seems
> handy.
Here's an updated diff that adds ^U (default control character for line
kill), documents the two hardcoded keys as such while removing the lie
about unimplemented word kill.

Is that any better?


Index: engine.c
===
RCS file: /cvs/src/usr.bin/systat/engine.c,v
retrieving revision 1.25
diff -u -p -r1.25 engine.c
--- engine.c12 Jan 2020 20:51:08 -  1.25
+++ engine.c22 Jun 2020 13:39:51 -
@@ -1204,6 +1204,7 @@ cmd_keyboard(int ch)
break;
case 0x1b:
case CTRL_G:
+   case CTRL_U:
if (cmd_len > 0) {
cmdbuf[0] = '\0';
cmd_len = 0;
Index: engine.h
===
RCS file: /cvs/src/usr.bin/systat/engine.h,v
retrieving revision 1.12
diff -u -p -r1.12 engine.h
--- engine.h12 Jan 2020 20:51:08 -  1.12
+++ engine.h22 Jun 2020 13:39:52 -
@@ -36,6 +36,7 @@
 #define CTRL_L  12
 #define CTRL_N  14
 #define CTRL_P  16
+#define CTRL_U  21
 #define CTRL_V  22
 
 #define META_V  246
Index: systat.1
===
RCS file: /cvs/src/usr.bin/systat/systat.1,v
retrieving revision 1.119
diff -u -p -r1.119 systat.1
--- systat.122 Jun 2020 13:17:54 -  1.119
+++ systat.123 Jun 2020 00:26:43 -
@@ -173,7 +173,7 @@ These are:
 Move the cursor to the command line and interpret the input
 line typed as a command.
 While entering a command the
-current character erase, word erase, and line kill characters
+character erase (^H) and line kill (^U) characters
 may be used.
 .It Ic o
 Select the next ordering which sorts the rows according to a



Re: userland clock_gettime proof of concept

2020-06-22 Thread Paul Irofti
New iteration:

  - ps_timekeep should not coredump, pointed by deraadt@
  - set ps_timekeep to 0 before user uvm_map for randomization
  - map timekeep before fixup. confirmed by naddy@ that it fixes NULL init
  - initialize va. clarified by kettenis@

How's the magical max skew value research going? Do we have a value yet?

Paul


diff --git lib/libc/arch/aarch64/gen/Makefile.inc 
lib/libc/arch/aarch64/gen/Makefile.inc
index a7b1b73f3ef..ee198f5d611 100644
--- lib/libc/arch/aarch64/gen/Makefile.inc
+++ lib/libc/arch/aarch64/gen/Makefile.inc
@@ -9,4 +9,4 @@ SRCS+=  fpgetmask.c fpgetround.c fpgetsticky.c
 SRCS+= fpsetmask.c fpsetround.c fpsetsticky.c
 SRCS+= fpclassifyl.c
 SRCS+= isfinitel.c isinfl.c isnanl.c isnormall.c
-SRCS+= signbitl.c
+SRCS+= signbitl.c usertc.c
diff --git lib/libc/arch/aarch64/gen/usertc.c lib/libc/arch/aarch64/gen/usertc.c
new file mode 100644
index 000..6551854a010
--- /dev/null
+++ lib/libc/arch/aarch64/gen/usertc.c
@@ -0,0 +1,21 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2020 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
+ * 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 
+
+int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
diff --git lib/libc/arch/alpha/gen/Makefile.inc 
lib/libc/arch/alpha/gen/Makefile.inc
index a44599d2cab..2a8abd32b61 100644
--- lib/libc/arch/alpha/gen/Makefile.inc
+++ lib/libc/arch/alpha/gen/Makefile.inc
@@ -3,5 +3,5 @@
 
 SRCS+= _setjmp.S fabs.S infinity.c ldexp.c modf.c nan.c setjmp.S
 SRCS+= flt_rounds.c fpgetmask.c fpgetround.c fpgetsticky.c fpsetmask.c \
-   fpsetround.c fpsetsticky.c
+   fpsetround.c fpsetsticky.c usertc.c
 SRCS+= sigsetjmp.S
diff --git lib/libc/arch/alpha/gen/usertc.c lib/libc/arch/alpha/gen/usertc.c
new file mode 100644
index 000..6551854a010
--- /dev/null
+++ lib/libc/arch/alpha/gen/usertc.c
@@ -0,0 +1,21 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2020 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
+ * 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 
+
+int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
diff --git lib/libc/arch/amd64/gen/Makefile.inc 
lib/libc/arch/amd64/gen/Makefile.inc
index e995309ed71..f6349e2b974 100644
--- lib/libc/arch/amd64/gen/Makefile.inc
+++ lib/libc/arch/amd64/gen/Makefile.inc
@@ -2,6 +2,7 @@
 
 SRCS+= _setjmp.S fabs.S infinity.c ldexp.c modf.S nan.c setjmp.S \
sigsetjmp.S
-SRCS+= fpclassifyl.c isfinitel.c isinfl.c isnanl.c isnormall.c signbitl.c
+SRCS+= fpclassifyl.c isfinitel.c isinfl.c isnanl.c isnormall.c signbitl.c \
+   usertc.c
 SRCS+= flt_rounds.S fpgetmask.S fpgetround.S fpgetsticky.S fpsetmask.S \
fpsetround.S fpsetsticky.S
diff --git lib/libc/arch/amd64/gen/usertc.c lib/libc/arch/amd64/gen/usertc.c
new file mode 100644
index 000..56016c8eca1
--- /dev/null
+++ lib/libc/arch/amd64/gen/usertc.c
@@ -0,0 +1,41 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2020 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
+ * 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 

net/if_pppx.c: add missing `IFF_RUNNING' bit routines

2020-06-22 Thread Vitaliy Makkoveev
pppx_if_start() checks `IFF_RUNNING' bit to be sure `pxi' is consistent
but this bit is not cleared on `pxi' destroy. pppx_if_output() checks
`IFF_UP' bit for the same reason but this bit is always set and this
check is useless. pppac_output() checks `IFF_RUNNING' bit but
pppac_start() doesn't.

Diff below adds `IFF_RUNNING' clearing to pppx_if_destroy() before `pxi'
destruction to be sure this `pxi' can't be accessed from `ifnet' in
potencial context switch caused by if_detach().

In pppx_if_output() `IFF_RUNNING' checked instead of `IFF_UP' for
consistency with other code.

Also `IFF_RUNNING' checked in pppac_start().

Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.89
diff -u -p -r1.89 if_pppx.c
--- sys/net/if_pppx.c   22 Jun 2020 10:01:03 -  1.89
+++ sys/net/if_pppx.c   22 Jun 2020 11:59:28 -
@@ -854,9 +854,10 @@ pppx_if_destroy(struct pppx_dev *pxd, st
struct pipex_session *session;
 
NET_ASSERT_LOCKED();
-   pxi->pxi_ready = 0;
session = pxi->pxi_session;
ifp = >pxi_if;
+   pxi->pxi_ready = 0;
+   CLR(ifp->if_flags, IFF_RUNNING);
 
pipex_unlink_session(session);
 
@@ -910,7 +911,7 @@ pppx_if_output(struct ifnet *ifp, struct
 
NET_ASSERT_LOCKED();
 
-   if (!ISSET(ifp->if_flags, IFF_UP)) {
+   if (!ISSET(ifp->if_flags, IFF_RUNNING)) {
m_freem(m);
error = ENETDOWN;
goto out;
@@ -1465,6 +1466,9 @@ pppac_start(struct ifnet *ifp)
 {
struct pppac_softc *sc = ifp->if_softc;
struct mbuf *m;
+
+   if (!ISSET(ifp->if_flags, IFF_RUNNING))
+   return;
 
while ((m = ifq_dequeue(>if_snd)) != NULL) {
 #if NBPFILTER > 0



Re: userland clock_gettime proof of concept

2020-06-22 Thread Theo de Raadt
Christian Weisgerber  wrote:

> Christian Weisgerber:
> 
> > If I move
> > 
> > vaddr_t ps_timekeep;/* User pointer to timekeep */
> > 
> > up into the zeroed area, I get a properly randomized _timekeep in
> > userland.
> 
> Also note that exec_sigcode_map() has this
> 
> pr->ps_sigcode = 0; /* no hint */
> uao_reference(e->e_sigobject);
> if (uvm_map(>ps_vmspace->vm_map, >ps_sigcode, round_page(sz),
> 
> I don't know if we want to
> * explicitly set ps_timekeep to 0 in exec_timekeep_map(), or
> * move it into the zeroed area, which we should also do with ps_sigcode
>   then.

Placing it in the zero'd area probably needs some careful consideration and
testing in relationship to MD signal delivery, we would not want
 tf->tf_pr = (int)p->p_p->ps_sigcode
to become a pointer to userland NULL, resulting in a fault, which sends
a signal and 

Looking at ps_sigcode use in the kernel, I also find a special case for it
in uvm_should_coredump().  The same "avoid dumping" logic should probably
be there for timekeep also.



Re: userland clock_gettime proof of concept

2020-06-22 Thread Paul Irofti
On Mon, Jun 22, 2020 at 09:46:13AM -0600, Theo de Raadt wrote:
> Christian Weisgerber  wrote:
> 
> > Paul Irofti:
> > 
> > > 683 /* map the process's timekeep page */
> > > 684 if (exec_timekeep_map(pr))
> > > 685 goto free_pack_abort;
> > > 686 /* setup new registers and do misc. setup. */
> > > 687 if (pack.ep_emul->e_fixup != NULL) {
> > > 688 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
> > > 689 goto free_pack_abort;
> > > 690 }
> > 
> > Yes, with this init(8) gets a proper _timekeep instead of 0x0.
> > 
> > For randomization of the userland page...
> > 
> > +   if (uvm_map(>ps_vmspace->vm_map, >ps_timekeep, 
> > round_page(timekeep_sz),
> > 
> > ... ps_timekeep need to be 0 here.  At the moment, it inherits the
> > value from the parent process in fork().
> > 
> > In struct process in sys/proc.h, there is this:
> > 
> > /* The following fields are all zeroed upon creation in process_new. */
> > ...
> > /* End area that is zeroed on creation. */
> > 
> > If I move
> > 
> > vaddr_t ps_timekeep;/* User pointer to timekeep */
> > 
> > up into the zeroed area, I get a properly randomized _timekeep in
> > userland.
> 
> Right.
> 
> 
> BTW, why is this important?  One could say this does not need to
> be randomized.  It has no secret.  But a significant downside occurs
> with visible effects.
> 
> If that 1 page is always in the same place, then address-space
> randomizated mappings of future objects will not be able to place an
> object over that one page.
> 
> The address space is significantly less randomized as soon as it
> contains one fixed object.  Less randomized in a severe way impacting
> security.

Fully agree. I am going to send a new diff out with all of these
included.



Re: userland clock_gettime proof of concept

2020-06-22 Thread Paul Irofti
On Mon, Jun 22, 2020 at 05:35:48PM +0200, Christian Weisgerber wrote:
> Paul Irofti:
> 
> > 683 /* map the process's timekeep page */
> > 684 if (exec_timekeep_map(pr))
> > 685 goto free_pack_abort;
> > 686 /* setup new registers and do misc. setup. */
> > 687 if (pack.ep_emul->e_fixup != NULL) {
> > 688 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
> > 689 goto free_pack_abort;
> > 690 }
> 
> Yes, with this init(8) gets a proper _timekeep instead of 0x0.
> 
> For randomization of the userland page...
> 
> +   if (uvm_map(>ps_vmspace->vm_map, >ps_timekeep, 
> round_page(timekeep_sz),
> 
> ... ps_timekeep need to be 0 here.  At the moment, it inherits the
> value from the parent process in fork().
> 
> In struct process in sys/proc.h, there is this:
> 
> /* The following fields are all zeroed upon creation in process_new. */
> ...
> /* End area that is zeroed on creation. */
> 
> If I move
> 
> vaddr_t ps_timekeep;/* User pointer to timekeep */
> 
> up into the zeroed area, I get a properly randomized _timekeep in
> userland.

Nice, I bet the other mapping suffers from the same problem, checking
now with what Theo said.



Re: userland clock_gettime proof of concept

2020-06-22 Thread Theo de Raadt
Christian Weisgerber  wrote:

> Paul Irofti:
> 
> > 683 /* map the process's timekeep page */
> > 684 if (exec_timekeep_map(pr))
> > 685 goto free_pack_abort;
> > 686 /* setup new registers and do misc. setup. */
> > 687 if (pack.ep_emul->e_fixup != NULL) {
> > 688 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
> > 689 goto free_pack_abort;
> > 690 }
> 
> Yes, with this init(8) gets a proper _timekeep instead of 0x0.
> 
> For randomization of the userland page...
> 
> +   if (uvm_map(>ps_vmspace->vm_map, >ps_timekeep, 
> round_page(timekeep_sz),
> 
> ... ps_timekeep need to be 0 here.  At the moment, it inherits the
> value from the parent process in fork().
> 
> In struct process in sys/proc.h, there is this:
> 
> /* The following fields are all zeroed upon creation in process_new. */
> ...
> /* End area that is zeroed on creation. */
> 
> If I move
> 
> vaddr_t ps_timekeep;/* User pointer to timekeep */
> 
> up into the zeroed area, I get a properly randomized _timekeep in
> userland.

Right.


BTW, why is this important?  One could say this does not need to
be randomized.  It has no secret.  But a significant downside occurs
with visible effects.

If that 1 page is always in the same place, then address-space
randomizated mappings of future objects will not be able to place an
object over that one page.

The address space is significantly less randomized as soon as it
contains one fixed object.  Less randomized in a severe way impacting
security.



Re: userland clock_gettime proof of concept

2020-06-22 Thread Christian Weisgerber
Christian Weisgerber:

> If I move
> 
> vaddr_t ps_timekeep;/* User pointer to timekeep */
> 
> up into the zeroed area, I get a properly randomized _timekeep in
> userland.

Also note that exec_sigcode_map() has this

pr->ps_sigcode = 0; /* no hint */
uao_reference(e->e_sigobject);
if (uvm_map(>ps_vmspace->vm_map, >ps_sigcode, round_page(sz),

I don't know if we want to
* explicitly set ps_timekeep to 0 in exec_timekeep_map(), or
* move it into the zeroed area, which we should also do with ps_sigcode
  then.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: userland clock_gettime proof of concept

2020-06-22 Thread Christian Weisgerber
Paul Irofti:

> 683 /* map the process's timekeep page */
> 684 if (exec_timekeep_map(pr))
> 685 goto free_pack_abort;
> 686 /* setup new registers and do misc. setup. */
> 687 if (pack.ep_emul->e_fixup != NULL) {
> 688 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
> 689 goto free_pack_abort;
> 690 }

Yes, with this init(8) gets a proper _timekeep instead of 0x0.

For randomization of the userland page...

+   if (uvm_map(>ps_vmspace->vm_map, >ps_timekeep, 
round_page(timekeep_sz),

... ps_timekeep need to be 0 here.  At the moment, it inherits the
value from the parent process in fork().

In struct process in sys/proc.h, there is this:

/* The following fields are all zeroed upon creation in process_new. */
...
/* End area that is zeroed on creation. */

If I move

vaddr_t ps_timekeep;/* User pointer to timekeep */

up into the zeroed area, I get a properly randomized _timekeep in
userland.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: userland clock_gettime proof of concept

2020-06-22 Thread Theo de Raadt
Paul Irofti  wrote:

> So I don't know why the address is not randomized, but I bet if I print
> pr->ps_sigcode somehow from userland, it will be the same.

I just checked that.

60 out of 68 processes have a unique UVA for sigtramp, the other 8 processes
are fork's without execve.



C99 initializers in wsdisplay_font struct definitions

2020-06-22 Thread Frederic Cambus
Hi tech@,

Here is a diff to use C99 initializers in wsdisplay_font struct
definitions for Spleen kernel fonts.

Comments? OK?

Index: sys/dev/wsfont/spleen12x24.h
===
RCS file: /cvs/src/sys/dev/wsfont/spleen12x24.h,v
retrieving revision 1.6
diff -u -p -r1.6 spleen12x24.h
--- sys/dev/wsfont/spleen12x24.h21 Jun 2020 19:03:29 -  1.6
+++ sys/dev/wsfont/spleen12x24.h22 Jun 2020 09:44:40 -
@@ -29,18 +29,18 @@
 static u_char spleen12x24_data[];
 
 struct wsdisplay_font spleen12x24 = {
-   "Spleen 12x24", /* typeface name */
-   0,  /* index */
-   ' ',/* firstchar */
-   256 - ' ',  /* numchars */
-   WSDISPLAY_FONTENC_ISO,  /* encoding */
-   12, /* width */
-   24, /* height */
-   2,  /* stride */
-   WSDISPLAY_FONTORDER_L2R,/* bit order */
-   WSDISPLAY_FONTORDER_L2R,/* byte order */
-   NULL,   /* cookie */
-   spleen12x24_data/* data */
+   .name   = "Spleen 12x24",
+   .index  = 0,
+   .firstchar  = ' ',
+   .numchars   = 256 - ' ',
+   .encoding   = WSDISPLAY_FONTENC_ISO,
+   .fontwidth  = 12,
+   .fontheight = 24,
+   .stride = 2,
+   .bitorder   = WSDISPLAY_FONTORDER_L2R,
+   .byteorder  = WSDISPLAY_FONTORDER_L2R,
+   .cookie = NULL,
+   .data   = spleen12x24_data
 };
 
 static u_char spleen12x24_data[] = {
Index: sys/dev/wsfont/spleen16x32.h
===
RCS file: /cvs/src/sys/dev/wsfont/spleen16x32.h,v
retrieving revision 1.5
diff -u -p -r1.5 spleen16x32.h
--- sys/dev/wsfont/spleen16x32.h21 Jun 2020 19:03:29 -  1.5
+++ sys/dev/wsfont/spleen16x32.h22 Jun 2020 09:44:40 -
@@ -29,18 +29,18 @@
 static u_char spleen16x32_data[];
 
 struct wsdisplay_font spleen16x32 = {
-   "Spleen 16x32", /* typeface name */
-   0,  /* index */
-   ' ',/* firstchar */
-   256 - ' ',  /* numchars */
-   WSDISPLAY_FONTENC_ISO,  /* encoding */
-   16, /* width */
-   32, /* height */
-   2,  /* stride */
-   WSDISPLAY_FONTORDER_L2R,/* bit order */
-   WSDISPLAY_FONTORDER_L2R,/* byte order */
-   NULL,   /* cookie */
-   spleen16x32_data/* data */
+   .name   = "Spleen 16x32",
+   .index  = 0,
+   .firstchar  = ' ',
+   .numchars   = 256 - ' ',
+   .encoding   = WSDISPLAY_FONTENC_ISO,
+   .fontwidth  = 16,
+   .fontheight = 32,
+   .stride = 2,
+   .bitorder   = WSDISPLAY_FONTORDER_L2R,
+   .byteorder  = WSDISPLAY_FONTORDER_L2R,
+   .cookie = NULL,
+   .data   = spleen16x32_data
 };
 
 static u_char spleen16x32_data[] = {
Index: sys/dev/wsfont/spleen32x64.h
===
RCS file: /cvs/src/sys/dev/wsfont/spleen32x64.h,v
retrieving revision 1.6
diff -u -p -r1.6 spleen32x64.h
--- sys/dev/wsfont/spleen32x64.h21 Jun 2020 19:03:29 -  1.6
+++ sys/dev/wsfont/spleen32x64.h22 Jun 2020 09:44:41 -
@@ -29,18 +29,18 @@
 static u_char spleen32x64_data[];
 
 struct wsdisplay_font spleen32x64 = {
-   "Spleen 32x64", /* typeface name */
-   0,  /* index */
-   ' ',/* firstchar */
-   256 - ' ',  /* numchars */
-   WSDISPLAY_FONTENC_ISO,  /* encoding */
-   32, /* width */
-   64, /* height */
-   4,  /* stride */
-   WSDISPLAY_FONTORDER_L2R,/* bit order */
-   WSDISPLAY_FONTORDER_L2R,/* byte order */
-   NULL,   /* cookie */
-   spleen32x64_data/* data */
+   .name   = "Spleen 32x64",
+   .index  = 0,
+   .firstchar  = ' ',
+   .numchars   = 256 - ' ',
+   .encoding   = WSDISPLAY_FONTENC_ISO,
+   .fontwidth  = 32,
+   .fontheight = 64,
+   .stride = 4,
+   .bitorder   = WSDISPLAY_FONTORDER_L2R,
+   .byteorder  = WSDISPLAY_FONTORDER_L2R,
+   .cookie = NULL,
+   .data   = spleen32x64_data
 };
 
 static u_char spleen32x64_data[] = {
Index: sys/dev/wsfont/spleen5x8.h

Re: userland clock_gettime proof of concept

2020-06-22 Thread Theo de Raadt
Paul Irofti  wrote:

> So I don't know why the address is not randomized, but I bet if I print
> pr->ps_sigcode somehow from userland, it will be the same.

echo kern.allowkmem=1 >> /etc/sysctl.conf
reboot

Then procmap can be run on all processes, to see what VA the sigtramp
and timekeep land at.

It should be an independent random VA in every execve'd process.



Re: systat.1: Remove ^z mention

2020-06-22 Thread Jason McIntyre
On Mon, Jun 22, 2020 at 03:12:34PM +0200, Klemens Nanni wrote:
> On Mon, Jun 22, 2020 at 07:22:24AM +0100, Jason McIntyre wrote:
> > i guess the diff is correct, but removes what might be a handy reminder.
> It makes assumptions about the environment in which systat was started
> and it is wrong/misleading in some cases.
> 
> > i'm fine with this bit of doc remaining or being deleted. i guess ^Z is
> > well enough known that people don;t need it documented here.
> Then top(1) ought to have it as well, but there is no point in doing so;
> we might as well mention ^c killing the program in most manuals, but
> even then we're making an assumption about the program's environment
> rather than documenting behaviour of the very program itself.
> 

yes, these are good points.
jmc



Re: systat.1: Remove ^z mention

2020-06-22 Thread Klemens Nanni
On Mon, Jun 22, 2020 at 07:22:24AM +0100, Jason McIntyre wrote:
> i guess the diff is correct, but removes what might be a handy reminder.
It makes assumptions about the environment in which systat was started
and it is wrong/misleading in some cases.

> i'm fine with this bit of doc remaining or being deleted. i guess ^Z is
> well enough known that people don;t need it documented here.
Then top(1) ought to have it as well, but there is no point in doing so;
we might as well mention ^c killing the program in most manuals, but
even then we're making an assumption about the program's environment
rather than documenting behaviour of the very program itself.



Re: install npppd.conf with mode 0600

2020-06-22 Thread Vitaliy Makkoveev
On Mon, Jun 22, 2020 at 02:57:53PM +0900, YASUOKA Masahiko wrote:
> The line in etc/mtree/special should be updated as well.
> 
>   npppd.conf  type=file mode=0640 uname=root gname=wheel
> 
> other than that, ok yasuoka

Thanks for pointing.



Re: userland clock_gettime proof of concept

2020-06-22 Thread Paul Irofti
On Sun, Jun 21, 2020 at 05:42:55PM -0600, Theo de Raadt wrote:
> Paul Irofti  wrote:
> 
> > 
> > 
> > 
> > ??n 22 iunie 2020 01:26:16 EEST, Christian Weisgerber  
> > a scris:
> > >Christian Weisgerber:
> > >
> > >> I tweaked the patch locally to make _timekeep a visible global
> > >> symbol in libc.
> > >> 
> > >> Printing its value has revealed two issues:
> > >> 
> > >> * The timekeep page is mapped to the same address for every process.
> > >>   It changes across reboots, but once running, it's always the same.
> > >>   kettenis suggested
> > >>   - vaddr_t va;
> > >>   + vaddr_t va = 0;
> > >>   in exec_timekeep_map(), but that doesn't make a difference.
> > >
> > >But that's the kernel mapping, and my observation concerns the
> > >userland mapping.  So based on this, I moved ps_timekeep up into
> > >the fields of struct process that are zeroed on creation.
> > >With that, _timekeep is always 0 for all processes. :-/
> > 
> > 
> > I don't understand what problem you are trying to solve. Is it that 
> > timekeep is the same? That's because we create only one page and the 
> > address gets copied on fork. The diff was not designed to have timekeep 
> > zero'd on every process so it doesn't account for it.
> 
> 
> And I think you aren't listening.
> 
> He is saying it is at the same VA in *every* userland process.  Since most
> processes do use this little system call execve, it is implausible for it
> to be at the same place, just like it is implausible for the signal tramp
> to be same place, or ld.so, or libc.

The code we are talking about is only called from inside this little
system call execve by exec_timekeep_map() and fixup().

683 /* setup new registers and do misc. setup. */
684 if (pack.ep_emul->e_fixup != NULL) {
685 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
686 goto free_pack_abort;
687 }
...
694 /* map the process's signal trampoline code */
695 if (exec_sigcode_map(pr, pack.ep_emul))
696 goto free_pack_abort;
697 /* map the process's timekeep page */
698 if (exec_timekeep_map(pr))
699 goto free_pack_abort;

The timekeep map code is doing the same thing as the sigcode map.

880 int
881 exec_timekeep_map(struct process *pr)
882 {
883 size_t timekeep_sz = sizeof(struct timekeep);
884
885 /*
886  * Similar to the sigcode object, except that there is a single
887  * timekeep object, and not one per emulation.
888  */
889 if (timekeep_object == NULL) {

The timekeep_object is checked if allocated and if not it does a 
uvm_map(kernel_map).
The timekeep_object is global so the if condition is only true once.
Then a second call to uvm_map() sends it to the process space.

Fixup is called before this, which I think is wrong now.

863 a->au_id = AUX_openbsd_timekeep;
864 a->au_v = p->p_p->ps_timekeep;
865 a++;

It should be map-fixup rather than fixup-map, right? But even reversing the
order leads to the same va address.

683 /* map the process's timekeep page */
684 if (exec_timekeep_map(pr))
685 goto free_pack_abort;
686 /* setup new registers and do misc. setup. */
687 if (pack.ep_emul->e_fixup != NULL) {
688 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
689 goto free_pack_abort;
690 }

So I don't know why the address is not randomized, but I bet if I print
pr->ps_sigcode somehow from userland, it will be the same.

Paul



Re: Blacklist Ericsson F5521GW from umass

2020-06-22 Thread Stuart Henderson
On 2020/06/22 14:10, Tobias Heider wrote:
> On Mon, Jun 22, 2020 at 02:01:43PM +0200, Tobias Heider wrote:
> > Hi,
> > 
> > I noticed that the ramdisk takes ages to boot on my T420.
> > It seems that without umodem in the kernel, umass tries to attach to my
> > Erricson F5521GW WAN modem and fails after a annoyingly long timeout.
> > 
> > The diff below should prevent umass from matching this device.
> > 
> > ok?
> > 
> 
> Whoops, small update because uq_match should be UMATCH_NONE.
> 
> diff --git a/sys/dev/usb/umass_quirks.c b/sys/dev/usb/umass_quirks.c
> index cf17d07ef96..ba80c4fb6a6 100644
> --- a/sys/dev/usb/umass_quirks.c
> +++ b/sys/dev/usb/umass_quirks.c
> @@ -473,6 +473,14 @@ const struct umass_quirk umass_quirks[] = {
>UMATCH_VENDOR_PRODUCT,
>NULL, NULL
>   },
> +
> + { { USB_VENDOR_ERICSSON, USB_PRODUCT_ERICSSON_F5521GW },
> +  UMASS_WPROTO_UNSPEC, UMASS_CPROTO_UNSPEC,
> +  0,
> +  0,
> +  UMATCH_NONE,
> +  NULL, NULL
> + },
>  };
>  
>  const struct umass_quirk *
> diff --git a/sys/dev/usb/usbdevs.h b/sys/dev/usb/usbdevs.h
> index 45fda88512f..e9d2b6e6169 100644
> --- a/sys/dev/usb/usbdevs.h
> +++ b/sys/dev/usb/usbdevs.h

Generally OK but don't commit directly to usbdevs.h.

Edit usbdevs, commit, run "make", then commit the generated files
usbdevs_data.h and usbdevs.h.

> @@ -427,6 +427,7 @@
>  #define  USB_VENDOR_AMBIT0x0bb2  /* Ambit Microsystems */
>  #define  USB_VENDOR_HTC  0x0bb4  /* HTC */
>  #define  USB_VENDOR_REALTEK  0x0bda  /* Realtek */
> +#define  USB_VENDOR_ERICSSON 0x0bdb  /* Ericsson  */
>  #define  USB_VENDOR_MEI  0x0bed  /* MEI */
>  #define  USB_VENDOR_ADDONICS20x0bf6  /* Addonics Technology 
> */
>  #define  USB_VENDOR_FSC  0x0bf8  /* Fujitsu Siemens Computers */
> @@ -1774,6 +1775,9 @@
>  #define  USB_PRODUCT_EPSON_DX60000x082e  /* Stylus 
> DX6000 */
>  #define  USB_PRODUCT_EPSON_DX40000x082f  /* Stylus 
> DX4000 */
>  
> +/* Ericsson products */
> +#define USB_PRODUCT_ERICSSON_F5521GW 0x1911  /* Wireless WAN */
> +
>  /* e-TEK Labs products */
>  #define  USB_PRODUCT_ETEK_1COM   0x8007  /* Serial */
>  
> 



Re: Blacklist Ericsson F5521GW from umass

2020-06-22 Thread Tobias Heider
On Mon, Jun 22, 2020 at 02:01:43PM +0200, Tobias Heider wrote:
> Hi,
> 
> I noticed that the ramdisk takes ages to boot on my T420.
> It seems that without umodem in the kernel, umass tries to attach to my
> Erricson F5521GW WAN modem and fails after a annoyingly long timeout.
> 
> The diff below should prevent umass from matching this device.
> 
> ok?
> 

Whoops, small update because uq_match should be UMATCH_NONE.

diff --git a/sys/dev/usb/umass_quirks.c b/sys/dev/usb/umass_quirks.c
index cf17d07ef96..ba80c4fb6a6 100644
--- a/sys/dev/usb/umass_quirks.c
+++ b/sys/dev/usb/umass_quirks.c
@@ -473,6 +473,14 @@ const struct umass_quirk umass_quirks[] = {
 UMATCH_VENDOR_PRODUCT,
 NULL, NULL
},
+
+   { { USB_VENDOR_ERICSSON, USB_PRODUCT_ERICSSON_F5521GW },
+UMASS_WPROTO_UNSPEC, UMASS_CPROTO_UNSPEC,
+0,
+0,
+UMATCH_NONE,
+NULL, NULL
+   },
 };
 
 const struct umass_quirk *
diff --git a/sys/dev/usb/usbdevs.h b/sys/dev/usb/usbdevs.h
index 45fda88512f..e9d2b6e6169 100644
--- a/sys/dev/usb/usbdevs.h
+++ b/sys/dev/usb/usbdevs.h
@@ -427,6 +427,7 @@
 #defineUSB_VENDOR_AMBIT0x0bb2  /* Ambit Microsystems */
 #defineUSB_VENDOR_HTC  0x0bb4  /* HTC */
 #defineUSB_VENDOR_REALTEK  0x0bda  /* Realtek */
+#defineUSB_VENDOR_ERICSSON 0x0bdb  /* Ericsson  */
 #defineUSB_VENDOR_MEI  0x0bed  /* MEI */
 #defineUSB_VENDOR_ADDONICS20x0bf6  /* Addonics Technology 
*/
 #defineUSB_VENDOR_FSC  0x0bf8  /* Fujitsu Siemens Computers */
@@ -1774,6 +1775,9 @@
 #defineUSB_PRODUCT_EPSON_DX60000x082e  /* Stylus 
DX6000 */
 #defineUSB_PRODUCT_EPSON_DX40000x082f  /* Stylus 
DX4000 */
 
+/* Ericsson products */
+#define USB_PRODUCT_ERICSSON_F5521GW   0x1911  /* Wireless WAN */
+
 /* e-TEK Labs products */
 #defineUSB_PRODUCT_ETEK_1COM   0x8007  /* Serial */
 



Blacklist Ericsson F5521GW from umass

2020-06-22 Thread Tobias Heider
Hi,

I noticed that the ramdisk takes ages to boot on my T420.
It seems that without umodem in the kernel, umass tries to attach to my
Erricson F5521GW WAN modem and fails after a annoyingly long timeout.

The diff below should prevent umass from matching this device.

ok?


diff --git a/sys/dev/usb/umass_quirks.c b/sys/dev/usb/umass_quirks.c
index cf17d07ef96..d00e4c59fd1 100644
--- a/sys/dev/usb/umass_quirks.c
+++ b/sys/dev/usb/umass_quirks.c
@@ -473,6 +473,14 @@ const struct umass_quirk umass_quirks[] = {
 UMATCH_VENDOR_PRODUCT,
 NULL, NULL
},
+
+   { { USB_VENDOR_ERICSSON, USB_PRODUCT_ERICSSON_F5521GW },
+UMASS_WPROTO_UNSPEC, UMASS_CPROTO_UNSPEC,
+0,
+0,
+UMATCH_VENDOR_PRODUCT,
+NULL, NULL
+   },
 };
 
 const struct umass_quirk *
diff --git a/sys/dev/usb/usbdevs.h b/sys/dev/usb/usbdevs.h
index 45fda88512f..e9d2b6e6169 100644
--- a/sys/dev/usb/usbdevs.h
+++ b/sys/dev/usb/usbdevs.h
@@ -427,6 +427,7 @@
 #defineUSB_VENDOR_AMBIT0x0bb2  /* Ambit Microsystems */
 #defineUSB_VENDOR_HTC  0x0bb4  /* HTC */
 #defineUSB_VENDOR_REALTEK  0x0bda  /* Realtek */
+#defineUSB_VENDOR_ERICSSON 0x0bdb  /* Ericsson  */
 #defineUSB_VENDOR_MEI  0x0bed  /* MEI */
 #defineUSB_VENDOR_ADDONICS20x0bf6  /* Addonics Technology 
*/
 #defineUSB_VENDOR_FSC  0x0bf8  /* Fujitsu Siemens Computers */
@@ -1774,6 +1775,9 @@
 #defineUSB_PRODUCT_EPSON_DX60000x082e  /* Stylus 
DX6000 */
 #defineUSB_PRODUCT_EPSON_DX40000x082f  /* Stylus 
DX4000 */
 
+/* Ericsson products */
+#define USB_PRODUCT_ERICSSON_F5521GW   0x1911  /* Wireless WAN */
+
 /* e-TEK Labs products */
 #defineUSB_PRODUCT_ETEK_1COM   0x8007  /* Serial */
 



Re: vlan and bridge panic with latest snapshot

2020-06-22 Thread Martin Ziemer
On Mon, Jun 22, 2020 at 11:11:47AM +0200, Claudio Jeker wrote:
> This crashes are because of wg(4) calling the interface ioctl handler
> without holding the netlock() this is not allowed.
> 
> As a quick fix this diff may work.
> -- 
> :wq Claudio
> 
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.609
> diff -u -p -r1.609 if.c
> --- net/if.c  22 Jun 2020 03:07:57 -  1.609
> +++ net/if.c  22 Jun 2020 09:06:42 -
> @@ -2220,13 +2221,6 @@ ifioctl(struct socket *so, u_long cmd, c
>   break;
>  
>   /* don't take NET_LOCK because i2c reads take a long time */
> - error = ((*ifp->if_ioctl)(ifp, cmd, data));
> - break;
> - case SIOCSWG:
> - case SIOCGWG:
> - /* Don't take NET_LOCK to allow wg(4) to continue to send and
> -  * receive packets while we're loading a large number of
> -  * peers. wg(4) uses its own lock to serialise access. */
>   error = ((*ifp->if_ioctl)(ifp, cmd, data));
>   break;
>  
> Index: net/if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 if_wg.c
> --- net/if_wg.c   21 Jun 2020 12:11:26 -  1.3
> +++ net/if_wg.c   22 Jun 2020 09:06:37 -
> @@ -2450,10 +2450,14 @@ wg_ioctl(struct ifnet *ifp, u_long cmd, 
>  
>   switch (cmd) {
>   case SIOCSWG:
> + NET_UNLOCK();
>   ret = wg_ioctl_set(sc, (struct wg_data_io *) data);
> + NET_LOCK();
>   break;
>   case SIOCGWG:
> + NET_UNLOCK();
>   ret = wg_ioctl_get(sc, (struct wg_data_io *) data);
> + NET_LOCK();
>   break;
>   /* Interface IOCTLs */
>   case SIOCSIFADDR:
> 
Tested the patch: It fixes the problems i encountered.

Thank you!



Re: userland clock_gettime proof of concept

2020-06-22 Thread Mark Kettenis
> Date: Mon, 22 Jun 2020 13:53:25 +0300
> From: Paul Irofti 
> 
> > Still uses uint instead of u_int in places.  Still has the pointless
> > extra NULL and 0 for timecounters in files that are otherwise
> 
> If you don't like uint, then let's fix what's in the tree in amd64
> (which is how uint got used in my diff too). OK?

That is correct (matches the type used in .

ok kettenis@

> diff --git sys/arch/amd64/amd64/tsc.c sys/arch/amd64/amd64/tsc.c
> index 7a1dcb4ad75..25c98180852 100644
> --- sys/arch/amd64/amd64/tsc.c
> +++ sys/arch/amd64/amd64/tsc.c
> @@ -42,7 +42,7 @@ int64_t tsc_drift_observed;
>  volatile int64_t tsc_sync_val;
>  volatile struct cpu_info *tsc_sync_cpu;
>  
> -uint tsc_get_timecount(struct timecounter *tc);
> +u_inttsc_get_timecount(struct timecounter *tc);
>  
>  #include "lapic.h"
>  #if NLAPIC > 0
> @@ -207,7 +207,7 @@ cpu_recalibrate_tsc(struct timecounter *tc)
>   calibrate_tsc_freq();
>  }
>  
> -uint
> +u_int
>  tsc_get_timecount(struct timecounter *tc)
>  {
>   return rdtsc() + curcpu()->ci_tsc_skew;
> 



Re: userland clock_gettime proof of concept

2020-06-22 Thread Paul Irofti
> Still uses uint instead of u_int in places.  Still has the pointless
> extra NULL and 0 for timecounters in files that are otherwise

If you don't like uint, then let's fix what's in the tree in amd64
(which is how uint got used in my diff too). OK?

diff --git sys/arch/amd64/amd64/tsc.c sys/arch/amd64/amd64/tsc.c
index 7a1dcb4ad75..25c98180852 100644
--- sys/arch/amd64/amd64/tsc.c
+++ sys/arch/amd64/amd64/tsc.c
@@ -42,7 +42,7 @@ int64_t   tsc_drift_observed;
 volatile int64_t   tsc_sync_val;
 volatile struct cpu_info   *tsc_sync_cpu;
 
-uint   tsc_get_timecount(struct timecounter *tc);
+u_int  tsc_get_timecount(struct timecounter *tc);
 
 #include "lapic.h"
 #if NLAPIC > 0
@@ -207,7 +207,7 @@ cpu_recalibrate_tsc(struct timecounter *tc)
calibrate_tsc_freq();
 }
 
-uint
+u_int
 tsc_get_timecount(struct timecounter *tc)
 {
return rdtsc() + curcpu()->ci_tsc_skew;



Re: userland clock_gettime proof of concept

2020-06-22 Thread Paul Irofti
On Mon, Jun 22, 2020 at 01:27:22AM +0200, Mark Kettenis wrote:
> > Date: Mon, 22 Jun 2020 02:06:39 +0300
> > From: Paul Irofti 
> > 
> > În 22 iunie 2020 00:15:59 EEST, Christian Weisgerber  a 
> > scris:
> > >Paul Irofti:
> > >
> > >[Unrelated, just to mark where we're at]
> > >> Right. Just reproduced it here. This moves the check at the top so
> > >that
> > >> each CPU checks its own skew and disables tc_user if necessary.
> > >
> > >I tweaked the patch locally to make _timekeep a visible global
> > >symbol in libc.
> > >
> > >Printing its value has revealed two issues:
> > >
> > >* The timekeep page is mapped to the same address for every process.
> > >  It changes across reboots, but once running, it's always the same.
> > >  kettenis suggested
> > >  - vaddr_t va;
> > >  + vaddr_t va = 0;
> > >  in exec_timekeep_map(), but that doesn't make a difference.
> > 
> > The va is set a few lines down the line. No point in
> > initialization. This is identical behavior to the emul mapping
> > before timekeep.
> 
> Well, uvm_map() picks a virtual address based on the value of va that
> is passed in.  If it is zero, it picks a random address.  If not, it
> uses the value as a hint and tries to pick something nearby.  So
> passing in stack garbage is a bad thing.

But uoffset=0 means it is not UVM_UNKNOWN_OFFSET (-1) and we have a
non-NULL uobj, so my understanding is that the va address is ignored in
this case. So it does not need to be initialized. Right?

  if (uvm_map(kernel_map, , round_page(timekeep_sz), timekeep_object,
  0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
  MAP_INHERIT_SHARE, MADV_RANDOM, 0))) {


None the less, I added va=0 in my diff. But I think it is pointless. If
you disagree, then do you OK the following diff?


diff --git sys/kern/kern_exec.c sys/kern/kern_exec.c
index 20480c2fc28..2b2b4f15222 100644
--- sys/kern/kern_exec.c
+++ sys/kern/kern_exec.c
@@ -828,7 +828,7 @@ exec_sigcode_map(struct process *pr, struct emul *e)
extern int sigfillsiz;
extern u_char sigfill[];
size_t off;
-   vaddr_t va;
+   vaddr_t va = 0;
int r;
 
e->e_sigobject = uao_create(sz, 0);



Re: vlan and bridge panic with latest snapshot

2020-06-22 Thread Stefan Sperling
On Mon, Jun 22, 2020 at 12:37:03PM +0200, Hrvoje Popovski wrote:
> for some reason i couldn't reproduce panic if i compile kernel with
> WITNESS and after that with or without your "if.c if_wg.c" commit 

The bug might not trigger if your ifconfig binary and kernel out of sync.
But that's just dumb luck.



Re: vlan and bridge panic with latest snapshot

2020-06-22 Thread Hrvoje Popovski
On 22.6.2020. 11:11, Claudio Jeker wrote:
> On Sun, Jun 21, 2020 at 08:51:53PM +0200, Hrvoje Popovski wrote:
>> Hi all,
>>
>> with today's snapshot from 21-Jun-2020 09:34
>> OpenBSD 6.7-current (GENERIC.MP) #286: Sun Jun 21 08:51:29 MDT 2020
>> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>>
>> if i do "ifconfig vlan" i'm getting assert
>> x3550m4# ifconfig vlan
>> vlan100: flags=8splassert: vlan_ioctl: want 2 have 0
>> Starting stack trace...
>> vlan_ioctl(80bb4800,c02069d3,800021f6f5d0) at vlan_ioctl+0x65
>> ifioctl(fd8785005668,c02069d3,800021f6f5d0,800021ffb130) at
>> ifioctl+0x91c
>> soo_ioctl(fd8784e6d630,c02069d3,800021f6f5d0,800021ffb130)
>> at soo_ioctl+0x171
>> sys_ioctl(800021ffb130,800021f6f6e0,800021f6f740) at
>> sys_ioctl+0x2df
>> syscall(800021f6f7b0) at syscall+0x389
>> Xsyscall() at Xsyscall+0x128
>> end of kernel
>> end trace frame: 0x7f7e53d0, count: 251
>> End of stack trace.
>>
>>
>> with ifconfig bridge0 up everything seems fine but ifconfig bridge0
>> destroy and ifconfig after that get me panic ..
>>
>> x3550m4# ifconfig
>> lo0: flags=8049 msplassert: vlan_ioctl:
>> want 2 have 0
>> Starting stack trace...
>> vlan_ioctl(80bb4800,c02069d3,800021f6f510) at vlan_ioctl+0x65
>> ifioctl(fd8785005668,c02069d3,800021f6f510,800021ffb130) at
>> ifioctl+0x91c
>> soo_ioctl(fd8784e6d630,c02069d3,800021f6f510,800021ffb130)
>> at soo_ioctl+0x171
>> sys_ioctl(800021ffb130,800021f6f620,800021f6f680) at
>> sys_ioctl+0x2df
>> syscall(800021f6f6f0) at syscall+0x389
>> Xsyscall() at Xsyscall+0x128
>> end of kernel
>> end trace frame: 0x7f7ddd20, count: 251
>> End of stack trace.
>> tu 32768
>> indexpanic: netlock: lock not held
>> Stopped at  db_enter+0x10:  popq%rbp
>> TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND
>> *505095   3193  0 0x3  03K ifconfig
>> db_enter() at db_enter+0x10
>> panic(81dbfaab) at panic+0x128
>> rw_exit_write(820e6138) at rw_exit_write+0xb5
>> bridge_ioctl(81754000,c02069d3,800021f6f510) at
>> bridge_ioctl+0x42
>> ifioctl(fd8785005668,c02069d3,800021f6f510,800021ffb130) at
>> ifioctl+0x91c
>> soo_ioctl(fd8784e6d630,c02069d3,800021f6f510,800021ffb130)
>> at soo_ioctl+0x171
>> sys_ioctl(800021ffb130,800021f6f620,800021f6f680) at
>> sys_ioctl+0x2df
>> syscall(800021f6f6f0) at syscall+0x389
>> Xsyscall() at Xsyscall+0x128
>> end of kernel
>> end trace frame: 0x7f7ddd20, count: 6
>> https://www.openbsd.org/ddb.html describes the minimum info required in
>> bugreports.  Insufficient info makes it difficult to find and fix bugs.
>>
> 
> This crashes are because of wg(4) calling the interface ioctl handler
> without holding the netlock() this is not allowed.
> 
> As a quick fix this diff may work.
> 

Hi,

for some reason i couldn't reproduce panic if i compile kernel with
WITNESS and after that with or without your "if.c if_wg.c" commit 

Thank you for fix ...




Re: vlan and bridge panic with latest snapshot

2020-06-22 Thread Claudio Jeker
On Sun, Jun 21, 2020 at 08:51:53PM +0200, Hrvoje Popovski wrote:
> Hi all,
> 
> with today's snapshot from 21-Jun-2020 09:34
> OpenBSD 6.7-current (GENERIC.MP) #286: Sun Jun 21 08:51:29 MDT 2020
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> 
> if i do "ifconfig vlan" i'm getting assert
> x3550m4# ifconfig vlan
> vlan100: flags=8splassert: vlan_ioctl: want 2 have 0
> Starting stack trace...
> vlan_ioctl(80bb4800,c02069d3,800021f6f5d0) at vlan_ioctl+0x65
> ifioctl(fd8785005668,c02069d3,800021f6f5d0,800021ffb130) at
> ifioctl+0x91c
> soo_ioctl(fd8784e6d630,c02069d3,800021f6f5d0,800021ffb130)
> at soo_ioctl+0x171
> sys_ioctl(800021ffb130,800021f6f6e0,800021f6f740) at
> sys_ioctl+0x2df
> syscall(800021f6f7b0) at syscall+0x389
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7f7e53d0, count: 251
> End of stack trace.
> 
> 
> with ifconfig bridge0 up everything seems fine but ifconfig bridge0
> destroy and ifconfig after that get me panic ..
> 
> x3550m4# ifconfig
> lo0: flags=8049 msplassert: vlan_ioctl:
> want 2 have 0
> Starting stack trace...
> vlan_ioctl(80bb4800,c02069d3,800021f6f510) at vlan_ioctl+0x65
> ifioctl(fd8785005668,c02069d3,800021f6f510,800021ffb130) at
> ifioctl+0x91c
> soo_ioctl(fd8784e6d630,c02069d3,800021f6f510,800021ffb130)
> at soo_ioctl+0x171
> sys_ioctl(800021ffb130,800021f6f620,800021f6f680) at
> sys_ioctl+0x2df
> syscall(800021f6f6f0) at syscall+0x389
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7f7ddd20, count: 251
> End of stack trace.
> tu 32768
> indexpanic: netlock: lock not held
> Stopped at  db_enter+0x10:  popq%rbp
> TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND
> *505095   3193  0 0x3  03K ifconfig
> db_enter() at db_enter+0x10
> panic(81dbfaab) at panic+0x128
> rw_exit_write(820e6138) at rw_exit_write+0xb5
> bridge_ioctl(81754000,c02069d3,800021f6f510) at
> bridge_ioctl+0x42
> ifioctl(fd8785005668,c02069d3,800021f6f510,800021ffb130) at
> ifioctl+0x91c
> soo_ioctl(fd8784e6d630,c02069d3,800021f6f510,800021ffb130)
> at soo_ioctl+0x171
> sys_ioctl(800021ffb130,800021f6f620,800021f6f680) at
> sys_ioctl+0x2df
> syscall(800021f6f6f0) at syscall+0x389
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7f7ddd20, count: 6
> https://www.openbsd.org/ddb.html describes the minimum info required in
> bugreports.  Insufficient info makes it difficult to find and fix bugs.
> 

This crashes are because of wg(4) calling the interface ioctl handler
without holding the netlock() this is not allowed.

As a quick fix this diff may work.
-- 
:wq Claudio

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.609
diff -u -p -r1.609 if.c
--- net/if.c22 Jun 2020 03:07:57 -  1.609
+++ net/if.c22 Jun 2020 09:06:42 -
@@ -2220,13 +2221,6 @@ ifioctl(struct socket *so, u_long cmd, c
break;
 
/* don't take NET_LOCK because i2c reads take a long time */
-   error = ((*ifp->if_ioctl)(ifp, cmd, data));
-   break;
-   case SIOCSWG:
-   case SIOCGWG:
-   /* Don't take NET_LOCK to allow wg(4) to continue to send and
-* receive packets while we're loading a large number of
-* peers. wg(4) uses its own lock to serialise access. */
error = ((*ifp->if_ioctl)(ifp, cmd, data));
break;
 
Index: net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.3
diff -u -p -r1.3 if_wg.c
--- net/if_wg.c 21 Jun 2020 12:11:26 -  1.3
+++ net/if_wg.c 22 Jun 2020 09:06:37 -
@@ -2450,10 +2450,14 @@ wg_ioctl(struct ifnet *ifp, u_long cmd, 
 
switch (cmd) {
case SIOCSWG:
+   NET_UNLOCK();
ret = wg_ioctl_set(sc, (struct wg_data_io *) data);
+   NET_LOCK();
break;
case SIOCGWG:
+   NET_UNLOCK();
ret = wg_ioctl_get(sc, (struct wg_data_io *) data);
+   NET_LOCK();
break;
/* Interface IOCTLs */
case SIOCSIFADDR:



Re: pipex(4): prevent `state_list' corruption

2020-06-22 Thread YASUOKA Masahiko
Yes, this seems right.

ok yasuoka

On Thu, 18 Jun 2020 23:53:25 +0300
Vitaliy Makkoveev  wrote:
> While pppac(4) destroy sessions by pipex_iface_fini() or by
> pipex_ioctl() with PIPEXSMODE command, some sessions can be linked to
> `state_list'. This case is not checked and sessions will never be
> unlinked and `state_list' will be broken after session's memory freeing.
> 
> Diff below adds session removal from `state_list' in
> pipex_unlink_session(). Also unlinked session `state' sets to
> PIPEX_STATE_CLOSED like pipex_close_session() does.
> 
> Index: sys/net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 pipex.c
> --- sys/net/pipex.c   18 Jun 2020 14:20:12 -  1.115
> +++ sys/net/pipex.c   18 Jun 2020 16:37:44 -
> @@ -473,8 +473,10 @@ pipex_unlink_session(struct pipex_sessio
>   break;
>   }
>  #endif
> -
> + if (session->state == PIPEX_STATE_CLOSE_WAIT)
> + LIST_REMOVE(session, state_list);
>   LIST_REMOVE(session, session_list);
> + session->state = PIPEX_STATE_CLOSED;
>  
>   /* if final session is destroyed, stop timer */
>   if (LIST_EMPTY(_session_list))
> 



Re: systat.1: Remove ^z mention

2020-06-22 Thread Jason McIntyre
On Mon, Jun 22, 2020 at 06:06:43AM +0200, Klemens Nanni wrote:
> Suspending systat with ^Z is done by the shell iff job control is
> enabled, not systat itself.
> 
> Try `set +m' to disable job control or start systat in a terminal
> without a shell, e.g. `xterm -e systat', to confirm that ^z does nothing
> in these cases.
> 
> Feedback? OK?
> 

i guess the diff is correct, but removes what might be a handy reminder.
i'm fine with this bit of doc remaining or being deleted. i guess ^Z is
well enough known that people don;t need it documented here.

jmc

> 
> Index: systat.1
> ===
> RCS file: /cvs/src/usr.bin/systat/systat.1,v
> retrieving revision 1.117
> diff -u -p -r1.117 systat.1
> --- systat.1  23 Apr 2020 07:57:27 -  1.117
> +++ systat.1  22 Jun 2020 04:02:06 -
> @@ -211,9 +210,6 @@ Scroll current view up by one line.
>  Scroll current view down by one page.
>  .It Ic Alt-V | Aq Ic Page Up
>  Scroll current view up by one page.
> -.It Ic ^Z
> -Suspend
> -.Nm .
>  .El
>  .Pp
>  The following commands are interpreted by the
> 



Re: obsd 6.7 - ntpd error msg

2020-06-22 Thread Otto Moerbeek
On Thu, Jun 18, 2020 at 11:41:17AM +0200, Otto Moerbeek wrote:

> On Thu, Jun 18, 2020 at 09:57:34AM +0200, Salvatore Cuzzilla wrote:
> 
> > Perfect, tnx!
> > 
> > On 18.06.2020 07:58, Otto Moerbeek wrote:
> > > On Wed, Jun 17, 2020 at 10:53:54PM +0200, Salvatore Cuzzilla wrote:
> > > 
> > > > Hi Otto here the logs (multitail) - @22:49:15 I restarted ntpd:
> > > > -
> > > > Jun 17 22:49:23 obsd ntpd[88568]: constraint reply from 188.61.106.24: 
> > > > offset -0.541051
> > > > Jun 17 22:49:46 obsd ntpd[88568]: peer 172.17.1.1 now valid
> > > > 01] /var/log/daemon  <---   
> > > > 
> > > > 
> > > > 
> > > >  248KB - 2020/06/17 22:49:46
> > > > -
> > > > Jun 17 14:00:01 obsd syslogd[80400]: restart
> > > > Jun 17 16:19:41 obsd ntpd[92699]: pipe write error (from main): No such 
> > > > file or directory
> > > > Jun 17 16:21:07 obsd ntpd[29588]: pipe write error (from main): No such 
> > > > file or directory
> > > > Jun 17 17:00:01 obsd syslogd[80400]: restart
> > > > Jun 17 17:01:25 obsd ntpd[96273]: pipe write error (from main): No such 
> > > > file or directory
> > > > Jun 17 17:02:38 obsd ntpd[94737]: pipe write error (from main): No such 
> > > > file or directory
> > > > Jun 17 20:00:01 obsd syslogd[80400]: restart
> > > > Jun 17 22:00:01 obsd syslogd[80400]: restart
> > > > Jun 17 22:49:22 obsd ntpd[40936]: pipe write error (from main): No such 
> > > > file or directory
> > > > 02] /var/log/messages <---  
> > > > 
> > > > 
> > > > 
> > > >  205KB - 2020/06/17 22:49:22
> > > > -
> > > > 22:49:15 -ksh ToTo@obsd ~ $ doas rcctl restart ntpd
> > > > ntpd(ok)
> > > > ntpd(ok)
> > > > 22:49:23 -ksh ToTo@obsd ~ $
> > > 
> > > 
> > > OK, now we're getting somewhere.  It always helps to provide lots of
> > > information form the start.
> > > 
> > > The message is generated by ntpd being stopped.  It is harmless,
> > > though it is actually wrong, it's a pip read error.
> > > 
> > > So nothing to worry about.  I'll see if the log level should be
> > > changed to debug for this one or maybe another solution.
> > > 
> 
> And now with diff.

I committed a slighlty more conservative version of this diff. A dns
read error (which should not happen) still logs at warn level.

-Otto

> 
> Index: ntp.c
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/ntp.c,v
> retrieving revision 1.164
> diff -u -p -r1.164 ntp.c
> --- ntp.c 11 Apr 2020 07:49:48 -  1.164
> +++ ntp.c 18 Jun 2020 09:39:03 -
> @@ -365,7 +365,7 @@ ntp_main(struct ntpd_conf *nconf, struct
>   if (nfds > 0 && pfd[PFD_PIPE_MAIN].revents & (POLLIN|POLLERR)) {
>   nfds--;
>   if (ntp_dispatch_imsg() == -1) {
> - log_warn("pipe write error (from main)");
> + log_debug("pipe read error (from main)");
>   ntp_quit = 1;
>   }
>   }
> @@ -380,7 +380,7 @@ ntp_main(struct ntpd_conf *nconf, struct
>   if (nfds > 0 && pfd[PFD_PIPE_DNS].revents & (POLLIN|POLLERR)) {
>   nfds--;
>   if (ntp_dispatch_imsg_dns() == -1) {
> - log_warn("pipe write error (from dns engine)");
> + log_debug("pipe read error (from dns engine)");
>   ntp_quit = 1;
>   }
>   }
> 



Re: systat.1: document "s" command

2020-06-22 Thread Jason McIntyre
On Mon, Jun 22, 2020 at 05:54:51AM +0200, Klemens Nanni wrote:
> Feedback? OK?
> 

ok.
jmc

> Index: systat.1
> ===
> RCS file: /cvs/src/usr.bin/systat/systat.1,v
> retrieving revision 1.117
> diff -u -p -r1.117 systat.1
> --- systat.1  23 Apr 2020 07:57:27 -  1.117
> +++ systat.1  22 Jun 2020 03:53:15 -
> @@ -188,6 +185,8 @@ Quit
>  .Nm .
>  .It Ic r
>  Reverse the selected ordering if supported by the view.
> +.It Ic s
> +Change the screen refresh interval in seconds.
>  .It Ic \&,
>  Print numbers with thousand separators, where applicable.
>  .It Ic ^A | Aq Ic Home
> 



Re: systat.1: Trim ":" description, support line kill character

2020-06-22 Thread Jason McIntyre
On Mon, Jun 22, 2020 at 05:49:43AM +0200, Klemens Nanni wrote:
> The manual's wording is untouched since import in 1995, engine.c however
> came to be in 2008 as "New display engine for systat" from canacar.
> 
> While characte erase (^h) works, word erase (^w) is not implemented and
> line kill (^u) is supported but as ^g instead.
> 
> I see no value in documenting this either way, so remove the lines from
> the manual but also support the actual line kill character for.
> 
> Feedback? OK?
> 

morning.

how will people be able to find this if we don;t document it? from a
brief skim of docs which may hold answers, i still can;t find where
these values are documented.

couldn;t we document these work and what the defaults are? it seems
handy.

jmc

> 
> Index: engine.c
> ===
> RCS file: /cvs/src/usr.bin/systat/engine.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 engine.c
> --- engine.c  12 Jan 2020 20:51:08 -  1.25
> +++ engine.c  22 Jun 2020 03:35:40 -
> @@ -1204,6 +1204,7 @@ cmd_keyboard(int ch)
>   break;
>   case 0x1b:
>   case CTRL_G:
> + case CTRL_U:
>   if (cmd_len > 0) {
>   cmdbuf[0] = '\0';
>   cmd_len = 0;
> Index: engine.h
> ===
> RCS file: /cvs/src/usr.bin/systat/engine.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 engine.h
> --- engine.h  12 Jan 2020 20:51:08 -  1.12
> +++ engine.h  22 Jun 2020 03:36:21 -
> @@ -36,6 +36,7 @@
>  #define CTRL_L  12
>  #define CTRL_N  14
>  #define CTRL_P  16
> +#define CTRL_U  21
>  #define CTRL_V  22
>  
>  #define META_V  246
> Index: systat.1
> ===
> RCS file: /cvs/src/usr.bin/systat/systat.1,v
> retrieving revision 1.117
> diff -u -p -r1.117 systat.1
> --- systat.1  23 Apr 2020 07:57:27 -  1.117
> +++ systat.1  22 Jun 2020 03:09:25 -
> @@ -172,9 +172,6 @@ These are:
>  .It Ic \&:
>  Move the cursor to the command line and interpret the input
>  line typed as a command.
> -While entering a command the
> -current character erase, word erase, and line kill characters
> -may be used.
>  .It Ic o
>  Select the next ordering which sorts the rows according to a
>  combination of columns.
>