Re: bc makefile tweak

2017-07-03 Thread David Gwynne

> On 4 Jul 2017, at 12:54, Michael W. Bombardieri  wrote:
> 
> Hi,
> 
> When building bc, yacc can directly write bc.c instead of renaming
> the file with mv. Does this look any better?

there's some debate at the moment about the least worst or best way of handling 
this kind of stuff.

it is definitely not correct to split the targets that yacc produces. making 
bc.c depend on y.tab.h without shell commands to generate it will make it 
impossible for make to get bc.c regenerated if bc.c has been deleted, but not 
y.tab.h.

cheers,
dlg

> 
> - Michael
> 
> 
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.bin/bc/Makefile,v
> retrieving revision 1.9
> diff -u -p -u -r1.9 Makefile
> --- Makefile  3 Jul 2017 00:17:52 -   1.9
> +++ Makefile  4 Jul 2017 02:50:46 -
> @@ -10,11 +10,10 @@ DPADD+=   ${LIBEDIT} ${LIBCURSES} ${LIBCR
> 
> .PATH:${.CURDIR}/../dc
> 
> -bc.c y.tab.h: bc.y
> - ${YACC} -d ${.CURDIR}/bc.y
> - mv y.tab.c bc.c
> +y.tab.h: bc.y
> + ${YACC} -o ${.CURDIR}/bc.c ${.CURDIR}/bc.y
> 
> -scan.o: y.tab.h
> +bc.c scan.o: y.tab.h
> 
> beforeinstall:
>   install -c -o ${BINOWN} -g ${BINGRP} -m 444 ${.CURDIR}/bc.library \
> 



Re: armv7 alignment faults

2017-07-03 Thread Theo de Raadt
> > i think i've noted about this before, around 13months ago freebsd
> > first disabled alignment faults, and they haven't enabled them since.
> > deja vu, or not, i don't recall if the last diff like below did go
> > anywhere, nor if it got discussed about, so i'm sorry in advance,
> > if i'm banging my head to the wall with this one.
> 
> AFAIK we're rather headed towards strict alignement rules whenever
> possible.

right.

fact: there are strict alignment architectures.
fact: there are non-strict alignment architectures, in fact now the
  majority by install base.

if we don't provide encouragement for developers to pay attention to
strict alignment architectures today, we might as well send them all
the ones we have to the recycler today, right?

for nearly 20 years I've spoken about the strengths we gain from
supporting diverse platforms.

as to whether we are headed to a world where all platforms are
non-strict?  in hardware?  I don't believe it.  In software, sure but
the emulation cost in hw or sw is never really free.

it comes with a complexity we all pay for, and it limits future
innovation.  so in some ways I'd prefer if we allow them to survive,
by continuing to demand a sw ecosystem that recognizes their
existance.

> They messed up their new sha1 implementation.  Should be fixed with
> git-2.13.1p0, git-2.13.2 works fine here.

and in this case, it found a bug which affected all the other strict
platforms.  I rest my case.



Re: armv7 alignment faults

2017-07-03 Thread Jeremie Courreges-Anglas
Artturi Alm  writes:

> Hi,
>
> i think i've noted about this before, around 13months ago freebsd
> first disabled alignment faults, and they haven't enabled them since.
> deja vu, or not, i don't recall if the last diff like below did go
> anywhere, nor if it got discussed about, so i'm sorry in advance,
> if i'm banging my head to the wall with this one.

AFAIK we're rather headed towards strict alignement rules whenever
possible.

> and indeed, even git is unusable without this (on latest snapshot w/git
> installed from packages), 'git branch' did happen to be the only command
> of the few i tried,
> that didn't end up into SIGBUS, but even it fails reliably w/-v..

They messed up their new sha1 implementation.  Should be fixed with
git-2.13.1p0, git-2.13.2 works fine here.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



bc makefile tweak

2017-07-03 Thread Michael W. Bombardieri
Hi,

When building bc, yacc can directly write bc.c instead of renaming
the file with mv. Does this look any better?

- Michael


Index: Makefile
===
RCS file: /cvs/src/usr.bin/bc/Makefile,v
retrieving revision 1.9
diff -u -p -u -r1.9 Makefile
--- Makefile3 Jul 2017 00:17:52 -   1.9
+++ Makefile4 Jul 2017 02:50:46 -
@@ -10,11 +10,10 @@ DPADD+= ${LIBEDIT} ${LIBCURSES} ${LIBCR
 
 .PATH: ${.CURDIR}/../dc
 
-bc.c y.tab.h: bc.y
-   ${YACC} -d ${.CURDIR}/bc.y
-   mv y.tab.c bc.c
+y.tab.h: bc.y
+   ${YACC} -o ${.CURDIR}/bc.c ${.CURDIR}/bc.y
 
-scan.o: y.tab.h
+bc.c scan.o: y.tab.h
 
 beforeinstall:
install -c -o ${BINOWN} -g ${BINGRP} -m 444 ${.CURDIR}/bc.library \



armv7 alignment faults

2017-07-03 Thread Artturi Alm
Hi,

i think i've noted about this before, around 13months ago freebsd
first disabled alignment faults, and they haven't enabled them since.
deja vu, or not, i don't recall if the last diff like below did go
anywhere, nor if it got discussed about, so i'm sorry in advance,
if i'm banging my head to the wall with this one.

and indeed, even git is unusable without this (on latest snapshot w/git
installed from packages), 'git branch' did happen to be the only command
of the few i tried,
that didn't end up into SIGBUS, but even it fails reliably w/-v..

# git branch
* av7_a4x_dsb
  av7_use_uvm
  master
# git branch -v
Bus error (core dumped) 

the diff below still id enough to get over those, and let the
software run regardless of it's assumptions about what is right or wrong.

$ git branch -v
* av7_a4x_dsb b065a9e59a0 [ahead 1] gotta try
  av7_use_uvm 7f8cb8f2d0d [behind 1] whitespace
  master  7f8cb8f2d0d whitespace

small quote of the related fbsd commit msg:
"The hardware we support has always been
able to do unaligned accesses, we've just never enabled it until now.

This brings FreeBSD into line with all the other major OSes, and should help
with the growing volume of 3rd-party software that assumes unaligned access
will just work on armv6 and armv7."

major or not, more usable w/o diving into ports would be much appreciated.
-Artturi


diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
index c91108e7066..716628750dd 100644
--- a/sys/arch/arm/arm/cpufunc.c
+++ b/sys/arch/arm/arm/cpufunc.c
@@ -396,7 +396,6 @@ armv7_setup()
| CPU_CONTROL_AFE;
 
cpuctrl = CPU_CONTROL_MMU_ENABLE
-   | CPU_CONTROL_AFLT_ENABLE
| CPU_CONTROL_DC_ENABLE
| CPU_CONTROL_BPRD_ENABLE
| CPU_CONTROL_IC_ENABLE



install.sub: Typo/whitespace nit

2017-07-03 Thread Klemens Nanni

Remove duplicate full stop and add space after function name.

Feedback/OK?

Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1019
diff -u -p -r1.1019 install.sub
--- install.sub 2 Jul 2017 12:45:43 -   1.1019
+++ install.sub 4 Jul 2017 00:11:59 -
@@ -275,7 +275,7 @@ diskinfo() {
_i=${_i##+([[:space:],])}
_i=${_i%%+([[:space:],])}

-   # Extract Network Address Authority information from dmesg..
+   # Extract Network Address Authority information from dmesg.
_n=$(dmesg | sed -En '/^'$_d' at /h;${g;s/^.* 
([a-z0-9]+\.[a-zA-Z0-9_]+)$/\1/p;}')

# Extract disk size from disklabel output.
@@ -2968,7 +2970,7 @@ do_install() {
finish_up
}

-do_upgrade(){
+do_upgrade() {
# Get $ROOTDISK and $ROOTDEV
get_rootinfo




Re: install.sub: Clean v[46]_info() ouput

2017-07-03 Thread Klemens Nanni

On Mon, Jul 03, 2017 at 10:47:31PM +, Robert Peichaer wrote:

Dokument explicitely possible outputs and tweak the sed expressions
to remove the superfluous whitespaces. I guess that does the trick.

Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1019
diff -u -p -p -u -r1.1019 install.sub
--- install.sub 2 Jul 2017 12:45:43 -   1.1019
+++ install.sub 3 Jul 2017 22:34:04 -
@@ -896,13 +896,16 @@ __EOT
}

# Obtain and output the inet information related to interface $1.
-# Should output '   '.
+# Outputs one of:
+# 
+# 
+# \n  [\n]
v4_info() {
ifconfig $1 inet | sed -n '
1s/.*

Re: install.sub: Clean v[46]_info() ouput

2017-07-03 Thread Robert Peichaer
On Wed, Jun 14, 2017 at 05:37:07PM +0200, Klemens Nanni wrote:
> With this patch, v[46]_info() both output exactly what their description
> says.
> 
> As of now, these functions are only used through
>   set -- $(v4_info $_if)
> which gracefully handles any constellation of whitespaces in the output
> just fine. However future usage might change (or not) and being precise
> about a function's output doesn't hurt.
> 
> The sed command itself is a tad clearer/smarter that way as well, imho.
> 
> Here's what I mean (whitespaces visualised):
> 
>   $ v4_info trunk0
>   UP
>   ??? ??192.168.1.20xff00??broadcast??192.168.1.255
>   $ v4_info_clean trunk0
>   UP
>   192.168.1.2??0xff00??broadcast??192.168.1.255
> 
> 
> Feeback/OK?
> 
> Index: install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1014
> diff -u -p -r1.1014 install.sub
> --- install.sub   3 Jun 2017 22:27:41 -   1.1014
> +++ install.sub   14 Jun 2017 15:13:05 -
> @@ -896,13 +896,14 @@ __EOT
> }
> 
> # Obtain and output the inet information related to interface $1.
> -# Should output '   '.
> +# Outputs '\n  '.
> v4_info() {
>   ifconfig $1 inet | sed -n '
> - 1s/.* - 1s/.*<.*/DOWN/p
> - /inet/s/netmask//
> - /inet/s///p'
> + 1{  s/.* + s/.*<.*/DOWN/p; }
> + /inet/{ s///
> + s/^[[:space:]]*//
> + s/netmask //p; }'
> }
> 
> # Convert a netmask in hex format ($1) to dotted decimal format.
> @@ -981,14 +982,15 @@ v4_config() {
> }
> 
> # Obtain and output the inet6 information related to interface $1.
> -# Should output ''.
> +# Outputs '\n  '.
> v6_info() {
>   ifconfig $1 inet6 | sed -n '
> - 1s/.* - 1s/.*<.*/DOWN/p
> - /scopeid/d
> - /inet6/s/prefixlen//
> - /inet6/s///p'
> + 1{  s/.* + s/.*<.*/DOWN/p; }
> + /inet6/{s///
> + /scopeid/d
> + s/^[[:space:]]*//
> + s/prefixlen //p; }'
> }
> 
> # Set up IPv6 default route on interface $1.

Dokument explicitely possible outputs and tweak the sed expressions
to remove the superfluous whitespaces. I guess that does the trick.

Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1019
diff -u -p -p -u -r1.1019 install.sub
--- install.sub 2 Jul 2017 12:45:43 -   1.1019
+++ install.sub 3 Jul 2017 22:34:04 -
@@ -896,13 +896,16 @@ __EOT
 }
 
 # Obtain and output the inet information related to interface $1.
-# Should output '   '.
+# Outputs one of:
+# 
+# 
+# \n  [\n]
 v4_info() {
ifconfig $1 inet | sed -n '
1s/.*

Re: serial console and ddb

2017-07-03 Thread Hrvoje Popovski
On 3.7.2017. 23:42, Stuart Henderson wrote:
> The phrase "break sequence" is often used, but it's a bit of a misnomer.
> When a serial port is connected but not actively transmitting data the tx
> line is usually held high. A "break" is when that line is low for more 
> than a frame duration (the length of which depends on the port speed).
> 
> If the tx line is only being held low for a short time during reboot,
> I wonder if you might get lucky by selecting a lower port speed.
> (Avoid going too low, especially if you have enabled extra debug
> messages, or it's likely to interfere with other kernel operations
> while it's printing to the console.)

Yes, on 38400 everything seems fine 

Thank you for nice info ...




Re: serial console and ddb

2017-07-03 Thread Stuart Henderson
On 2017/07/03 21:05, Hrvoje Popovski wrote:
> Hi all,
> 
> i'm having two firewalls fw1 and fw2 and on fw1 i'm sending console
> output to com0.
> 
> root@fw1:~
> # cat /etc/boot.conf
> stty com0 115200
> set tty com0
> 
> root@fw1:~
> # cat /etc/ttys | grep tty00
> tty00   "/usr/libexec/getty std.115200" vt220   on  secure
> 
> 
> on fw2 i'm using "cu -s 115200" to play with wonderful net MP stuff on
> fw1 :)
> 
> on fw1 in sysctl i'm having ddb.console=1 ... and here's funny part...
> 
> when i reboot fw2 and while fw2 is booting at some point it seems that
> fw2 send some break sequence to fw1 and fw1 drops to ddb prompt.
> when i put ddb.console=0 everything seems fine ...
> 
> i don't know is this feature or bug :)

The phrase "break sequence" is often used, but it's a bit of a misnomer.
When a serial port is connected but not actively transmitting data the tx
line is usually held high. A "break" is when that line is low for more 
than a frame duration (the length of which depends on the port speed).

If the tx line is only being held low for a short time during reboot,
I wonder if you might get lucky by selecting a lower port speed.
(Avoid going too low, especially if you have enabled extra debug
messages, or it's likely to interfere with other kernel operations
while it's printing to the console.)



ifstated diff rename variables to avoid state confusion

2017-07-03 Thread Rob Pierce
ifstated monitors interface state and the return state of invoked commands,
and takes action accordingly, all of which is managed with the help of a
finite state machine. That makes for a lot of "state" references in the code.

The following diff renames variables to make a distinction between link status
and command execution status (now referenced as "status"), and the various 
states
within the state machine (referred to as "state").

As an example of the current confusion, the link_states() function in parse.y
actually links states within the finite state machine (as oppose to having
anything to do with interface link states).

This diff helps to make my head hurt less.

For your consideration.

Regards,

Rob

Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.49
diff -u -p -r1.49 ifstated.c
--- ifstated.c  3 Jul 2017 18:45:34 -   1.49
+++ ifstated.c  3 Jul 2017 19:59:43 -
@@ -18,7 +18,7 @@
  */
 
 /*
- * ifstated listens to link_state transitions on interfaces
+ * ifstated listens to link status transitions on interfaces
  * and executes predefined commands.
  */
 
@@ -62,9 +62,9 @@ void  external_handler(int, short, void 
 void   external_exec(struct ifsd_external *, int);
 void   check_external_status(struct ifsd_state *);
 void   external_evtimer_setup(struct ifsd_state *, int);
-void   scan_ifstate(int, int, int);
-intscan_ifstate_single(int, int, struct ifsd_state *);
-void   fetch_state(void);
+void   scan_ifstatus(int, int, int);
+intscan_ifstatus_single(int, int, struct ifsd_state *);
+void   fetch_ifstatus(void);
 __dead voidusage(void);
 void   adjust_expressions(struct ifsd_expression_list *, int);
 void   adjust_external_expressions(struct ifsd_state *);
@@ -208,7 +208,7 @@ load_config(void)
clear_config(conf);
conf = newconf;
conf->initstate.entered = time(NULL);
-   fetch_state();
+   fetch_ifstatus();
external_evtimer_setup(>initstate, IFSD_EVTIMER_ADD);
adjust_external_expressions(>initstate);
eval_state(>initstate);
@@ -246,7 +246,7 @@ rt_msg_handler(int fd, short event, void
return;
 
memcpy(, rtm, sizeof(ifm));
-   scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1);
+   scan_ifstatus(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1);
 }
 
 void
@@ -419,37 +419,37 @@ external_evtimer_setup(struct ifsd_state
 #defineLINK_STATE_IS_DOWN(_s)  (!LINK_STATE_IS_UP((_s)))
 
 int
-scan_ifstate_single(int ifindex, int s, struct ifsd_state *state)
+scan_ifstatus_single(int ifindex, int s, struct ifsd_state *state)
 {
-   struct ifsd_ifstate *ifstate;
+   struct ifsd_ifstatus *ifstatus;
struct ifsd_expression_list expressions;
int changed = 0;
 
TAILQ_INIT();
 
-   TAILQ_FOREACH(ifstate, >interface_states, entries) {
-   if (ifstate->ifindex == ifindex) {
-   if (ifstate->prevstate != s &&
-   (ifstate->prevstate != -1 || !opt_inhibit)) {
+   TAILQ_FOREACH(ifstatus, >interfaces_status, entries) {
+   if (ifstatus->ifindex == ifindex) {
+   if (ifstatus->prevstatus != s &&
+   (ifstatus->prevstatus != -1 || !opt_inhibit)) {
struct ifsd_expression *expression;
int truth;
 
truth =
-   (ifstate->ifstate == IFSD_LINKUNKNOWN &&
+   (ifstatus->ifstatus == IFSD_LINKUNKNOWN &&
s == LINK_STATE_UNKNOWN) ||
-   (ifstate->ifstate == IFSD_LINKDOWN &&
+   (ifstatus->ifstatus == IFSD_LINKDOWN &&
LINK_STATE_IS_DOWN(s)) ||
-   (ifstate->ifstate == IFSD_LINKUP &&
+   (ifstatus->ifstatus == IFSD_LINKUP &&
LINK_STATE_IS_UP(s));
 
TAILQ_FOREACH(expression,
-   >expressions, entries) {
+   >expressions, entries) {
expression->truth = truth;
TAILQ_INSERT_TAIL(,
expression, eval);
changed = 1;
}
-   ifstate->prevstate = s;
+   ifstatus->prevstatus = s;
}
}
}
@@ -460,15 +460,15 @@ scan_ifstate_single(int ifindex, int s, 
 }
 
 void
-scan_ifstate(int 

Re: magic.5: Add missing types

2017-07-03 Thread Klemens Nanni

On Mon, Jul 03, 2017 at 05:36:52PM +0100, Nicholas Marriott wrote:

Hi

On Thu, Jun 29, 2017 at 09:29:57PM +0200, Klemens Nanni wrote:

[...]
What about the current version being 4.21? We're clearly ahead of this,
it seems magic(5) wasn't updated when nicm@ reimplemented things.

This patch documents the respective types.


Did you copy this text from upstream or write it?

It's copied from magic.5 provided by devel/libmagic-5.31.


Some comments inline.


Feedback/OK?

Index: magic.5
===
RCS file: /cvs/src/usr.bin/file/magic.5,v
retrieving revision 1.17
diff -u -p -r1.17 magic.5
--- magic.5 24 Apr 2016 07:02:07 -  1.17
+++ magic.5 29 Jun 2017 17:41:56 -
@@ -218,6 +218,31 @@ This is intended to be used with the tes
.Em x
(which is always true) and a message that is to be used if there are
no other matches.
+.It Dv clear
+This test is always true and clears the match flag for that continuation
+level.


I would just say "that level" or "the test's level" not "that
continuation level" because there is no previous mention of
"continuation".


+It is intended to be used with the default test.
+.It Dv name
+Define a
+.Dq named


I don't think quotation marks are necessary here.


+magic instance that can be called from another
+.Dv use
+magic entry, like a subroutine call.
+Named instance direct magic offsets are relative to the offset of the
+previous matched entry, but indirect offsets are relative to the
+beginning of the file as usual.
+Named magic entries always match.
+.It Dv use
+Recursively call the named magic starting from the current offset.
+If the name of the referenced begins with a


"the referenced instance" would be better than "the referenced".


Sounds good to me.



ifstated whitespace diff

2017-07-03 Thread Rob Pierce
Fix some variable alignment whitespace.

Rob

Index: ifstated.h
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v
retrieving revision 1.15
diff -u -p -r1.15 ifstated.h
--- ifstated.h  2 Jul 2017 15:28:26 -   1.15
+++ ifstated.h  3 Jul 2017 19:40:21 -
@@ -92,9 +92,9 @@ struct ifsd_expression {
} u;
int  depth;
u_int32_ttype;
-#define IFSD_OPER_AND  1
-#define IFSD_OPER_OR   2
-#define IFSD_OPER_NOT  3
+#define IFSD_OPER_AND  1
+#define IFSD_OPER_OR   2
+#define IFSD_OPER_NOT  3
 #define IFSD_OPER_EXTERNAL 4
 #define IFSD_OPER_IFSTATE  5
u_int8_t truth;



Re: serial console and ddb

2017-07-03 Thread Mark Kettenis
> From: Hrvoje Popovski 
> Date: Mon, 3 Jul 2017 21:05:01 +0200
> 
> Hi all,
> 
> i'm having two firewalls fw1 and fw2 and on fw1 i'm sending console
> output to com0.
> 
> root@fw1:~
> # cat /etc/boot.conf
> stty com0 115200
> set tty com0
> 
> root@fw1:~
> # cat /etc/ttys | grep tty00
> tty00   "/usr/libexec/getty std.115200" vt220   on  secure
> 
> 
> on fw2 i'm using "cu -s 115200" to play with wonderful net MP stuff on
> fw1 :)
> 
> on fw1 in sysctl i'm having ddb.console=1 ... and here's funny part...
> 
> when i reboot fw2 and while fw2 is booting at some point it seems that
> fw2 send some break sequence to fw1 and fw1 drops to ddb prompt.
> when i put ddb.console=0 everything seems fine ...
> 
> i don't know is this feature or bug :)

Unfortunately badly designed hardware tends to do this.

> ddb{0}> show panic
> the kernel did not panic
> 
> 
> ddb{0}> trace
> db_enter(3f8,0,8120bffa,10,800020942c78,286) at db_enter+0x9
> comintr(804ab000,80484d00,800020942d88,1,819a2de0,f
> fff80434e80) at comintr+0x263
> intr_handler(800020942d88,80434e80,0,80430a00,0,9)
> at intr_
> handler+0x6f
> Xintr_ioapic_edge4() at Xintr_ioapic_edge4+0xc9
> --- interrupt ---
> acpicpu_idle(8000e470,819a2de0,819a2df0,0,2b39538b2ae4a
> 2bd,286) at acpicpu_idle+0x242
> cpu_idle_cycle(819a2de0,2a2,0,819a2de0,8153c300,0)
> at c
> pu_idle_cycle+0x10
> end trace frame: 0x0, count: -6
> ddb{0}>
> 
> 
> ddb{0}> ps
>PID TID   PPIDUID  S   FLAGS  WAIT  COMMAND
>   7884   58547  97227  0  30x100083  ttyin ksh
>  97227   68404  42285   1000  30x10008b  pause ksh
>  42285  462270  70007   1000  30x90  selectsshd
>  70007  366696  42175  0  30x82  poll  sshd
>  12377  204501   4833 68  30x90  selectisakmpd
>   4833  361757  1  0  30x80  netio isakmpd
>  35330  141196  81120  0  30x100083  ttyin ksh
>  81120  186861  99676   1000  30x10008b  pause ksh
>  99676  194692  84798   1000  30x90  selectsshd
>  84798  341107  42175  0  30x82  poll  sshd
>  74439  375429  89626606  30x90  kqreadladvd
>  89626  327310  1  0  30x80  kqreadladvd
>  61156  173029  1  0  30x100083  ttyin ksh
>  69646  359364  1  0  30x100083  ttyin getty
>  24831  485313  1  0  30x100083  ttyin getty
>  91314   50076  1  0  30x100083  ttyin getty
>   2000  320701  1  0  30x100083  ttyin getty
>  31108  172209  1  0  30x100083  ttyin ksh
>  20670   84974  1  0  30x100098  poll  cron
>  490198325  85011 95  30x100092  kqreadsmtpd
>  23296  311537  85011103  30x100092  kqreadsmtpd
>  55584  511240  85011 95  30x100092  kqreadsmtpd
>  35318  468152  85011 95  30x100092  kqreadsmtpd
>  62158   26960  85011 95  30x100092  kqreadsmtpd
>  99316  206735  85011 95  30x100092  kqreadsmtpd
>  85011  169871  1  0  30x100080  kqreadsmtpd
>  42175  473577  1  0  30x80  selectsshd
>  63878   52970  63236 83  30x100092  poll  ntpd
>  63236  471563  87173 83  30x100092  poll  ntpd
>  87173  400722  1  0  30x100080  poll  ntpd
>  94958  312911  16156 74  30x100090  bpf   pflogd
>  16156  348728  1  0  30x100080  netio pflogd
>  59623  441013  22590 73  30x100090  kqreadsyslogd
>  22590  131486  1  0  30x100082  netio syslogd
>  54640  387075  0  0  3 0x14200  pgzerozerothread
>   7129  108777  0  0  3 0x14200  aiodoned  aiodoned
>  98767  424884  0  0  3 0x14200  syncerupdate
>  78627  423389  0  0  3 0x14200  cleaner   cleaner
>  84685  379372  0  0  3 0x14200  reaperreaper
>  44237   80666  0  0  3 0x14200  pgdaemon  pagedaemon
>  27951   68797  0  0  3 0x14200  bored crynlk
>  77707  278117  0  0  3 0x14200  bored crypto
>  23962  391218  0  0  3 0x14200  pftm  pfpurge
>  90317  414156  0  0  3 0x14200  usbtskusbtask
>  172318781  0  0  3 0x14200  usbatsk   usbatsk
>  27192  165218  0  0  3  0x40014200  acpi0 acpi0
>  79629  515002  0  0  7  0x40014200idle5
>  15440   67305  0  0  7  0x40014200idle4
>  20528  342900  0  0  7  0x40014200idle3
>  70659  319916  0  0  7  0x40014200idle2
>  30844  491566  0 

serial console and ddb

2017-07-03 Thread Hrvoje Popovski
Hi all,

i'm having two firewalls fw1 and fw2 and on fw1 i'm sending console
output to com0.

root@fw1:~
# cat /etc/boot.conf
stty com0 115200
set tty com0

root@fw1:~
# cat /etc/ttys | grep tty00
tty00   "/usr/libexec/getty std.115200" vt220   on  secure


on fw2 i'm using "cu -s 115200" to play with wonderful net MP stuff on
fw1 :)

on fw1 in sysctl i'm having ddb.console=1 ... and here's funny part...

when i reboot fw2 and while fw2 is booting at some point it seems that
fw2 send some break sequence to fw1 and fw1 drops to ddb prompt.
when i put ddb.console=0 everything seems fine ...

i don't know is this feature or bug :)


ddb{0}> show panic
the kernel did not panic


ddb{0}> trace
db_enter(3f8,0,8120bffa,10,800020942c78,286) at db_enter+0x9
comintr(804ab000,80484d00,800020942d88,1,819a2de0,f
fff80434e80) at comintr+0x263
intr_handler(800020942d88,80434e80,0,80430a00,0,9)
at intr_
handler+0x6f
Xintr_ioapic_edge4() at Xintr_ioapic_edge4+0xc9
--- interrupt ---
acpicpu_idle(8000e470,819a2de0,819a2df0,0,2b39538b2ae4a
2bd,286) at acpicpu_idle+0x242
cpu_idle_cycle(819a2de0,2a2,0,819a2de0,8153c300,0)
at c
pu_idle_cycle+0x10
end trace frame: 0x0, count: -6
ddb{0}>


ddb{0}> ps
   PID TID   PPIDUID  S   FLAGS  WAIT  COMMAND
  7884   58547  97227  0  30x100083  ttyin ksh
 97227   68404  42285   1000  30x10008b  pause ksh
 42285  462270  70007   1000  30x90  selectsshd
 70007  366696  42175  0  30x82  poll  sshd
 12377  204501   4833 68  30x90  selectisakmpd
  4833  361757  1  0  30x80  netio isakmpd
 35330  141196  81120  0  30x100083  ttyin ksh
 81120  186861  99676   1000  30x10008b  pause ksh
 99676  194692  84798   1000  30x90  selectsshd
 84798  341107  42175  0  30x82  poll  sshd
 74439  375429  89626606  30x90  kqreadladvd
 89626  327310  1  0  30x80  kqreadladvd
 61156  173029  1  0  30x100083  ttyin ksh
 69646  359364  1  0  30x100083  ttyin getty
 24831  485313  1  0  30x100083  ttyin getty
 91314   50076  1  0  30x100083  ttyin getty
  2000  320701  1  0  30x100083  ttyin getty
 31108  172209  1  0  30x100083  ttyin ksh
 20670   84974  1  0  30x100098  poll  cron
 490198325  85011 95  30x100092  kqreadsmtpd
 23296  311537  85011103  30x100092  kqreadsmtpd
 55584  511240  85011 95  30x100092  kqreadsmtpd
 35318  468152  85011 95  30x100092  kqreadsmtpd
 62158   26960  85011 95  30x100092  kqreadsmtpd
 99316  206735  85011 95  30x100092  kqreadsmtpd
 85011  169871  1  0  30x100080  kqreadsmtpd
 42175  473577  1  0  30x80  selectsshd
 63878   52970  63236 83  30x100092  poll  ntpd
 63236  471563  87173 83  30x100092  poll  ntpd
 87173  400722  1  0  30x100080  poll  ntpd
 94958  312911  16156 74  30x100090  bpf   pflogd
 16156  348728  1  0  30x100080  netio pflogd
 59623  441013  22590 73  30x100090  kqreadsyslogd
 22590  131486  1  0  30x100082  netio syslogd
 54640  387075  0  0  3 0x14200  pgzerozerothread
  7129  108777  0  0  3 0x14200  aiodoned  aiodoned
 98767  424884  0  0  3 0x14200  syncerupdate
 78627  423389  0  0  3 0x14200  cleaner   cleaner
 84685  379372  0  0  3 0x14200  reaperreaper
 44237   80666  0  0  3 0x14200  pgdaemon  pagedaemon
 27951   68797  0  0  3 0x14200  bored crynlk
 77707  278117  0  0  3 0x14200  bored crypto
 23962  391218  0  0  3 0x14200  pftm  pfpurge
 90317  414156  0  0  3 0x14200  usbtskusbtask
 172318781  0  0  3 0x14200  usbatsk   usbatsk
 27192  165218  0  0  3  0x40014200  acpi0 acpi0
 79629  515002  0  0  7  0x40014200idle5
 15440   67305  0  0  7  0x40014200idle4
 20528  342900  0  0  7  0x40014200idle3
 70659  319916  0  0  7  0x40014200idle2
 30844  491566  0  0  7  0x40014200idle1
 77845  252780  0  0  3 0x14200  bored sensors
 30764   5  0  0  3 0x14200  bored softnet
 59884  521485  0  0  3 0x14200  bored systqmp
 90717  141157  0  0  3 0x14200  bored systq
 77096   49912  0  0  

Re: ifstated readability diff

2017-07-03 Thread Sebastian Benoit
commited, thanks.

Rob Pierce(r...@2keys.ca) on 2017.07.03 09:45:35 -0400:
> On Sun, Jul 02, 2017 at 11:50:56PM -0400, Rob Pierce wrote:
> > Remove obvious clear_config() comments and misleading state_change() 
> > comments.
> > 
> > Also relocate do_action() calls for the init block from change_state() to
> > occur with the corresponding do_action() calls for the body block within
> > the calling function for improved readability.
> > 
> > No functional change.
> > 
> > Rob
> 
> That diff was broken. This one is correct.
> 
> Rob
> 
> Index: ifstated.c
> ===
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 ifstated.c
> --- ifstated.c2 Jul 2017 15:28:26 -   1.48
> +++ ifstated.c3 Jul 2017 13:42:13 -
> @@ -217,8 +217,10 @@ load_config(void)
>   conf->curstate->entered = time(NULL);
>   conf->nextstate = conf->curstate;
>   conf->curstate = NULL;
> - while (state_change())
> + while (state_change()) {
> + do_action(conf->curstate->init);
>   do_action(conf->curstate->body);
> + }
>   }
>   return (0);
>  }
> @@ -533,14 +535,13 @@ eval_state(struct ifsd_state *state)
>   if (external == NULL || external->lastexec >= state->entered ||
>   external->lastexec == 0) {
>   do_action(state->body);
> - while (state_change())
> + while (state_change()) {
> + do_action(conf->curstate->init);
>   do_action(conf->curstate->body);
> + }
>   }
>  }
>  
> -/*
> - *If a previous action included a state change, process it.
> - */
>  int
>  state_change(void)
>  {
> @@ -556,7 +557,6 @@ state_change(void)
>   conf->curstate->entered = time(NULL);
>   external_evtimer_setup(conf->curstate, IFSD_EVTIMER_ADD);
>   adjust_external_expressions(conf->curstate);
> - do_action(conf->curstate->init);
>   return (1);
>   }
>   return (0);
> @@ -627,9 +627,6 @@ fetch_state(void)
>   close(sock);
>  }
>  
> -/*
> - * Clear the config.
> - */
>  void
>  clear_config(struct ifsd_config *oconf)
>  {
> 



elf.h

2017-07-03 Thread Karel Gardas

 Hello,

I'm curious if it's possible to provide /usr/include/elf.h file on OpenBSD to 
improve its niceness to software porting from other Unixes. Following patch 
adds this for me and is tested with GHC where I'd like to kill code like:

#if !defined(openbsd_HOST_OS)
#  include 
#else
/* openbsd elf has things in different places, with diff names */
#  include 
#endif

I assume other apps dealing with ELF needs to do the similar thing for OpenBSD 
support. I've checked and /usr/include/elf.h is available on Solaris 10, 
Solaris 11, various Linuxes, NetBSD 7.x and FreeBSD 10.x. In patch below, elf.h 
is a result of copy of elf_abi.h with small edit.

Thanks,
Karel

diff --git a/include/Makefile b/include/Makefile
index 1d7f894b595..f8d8f141af2 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -11,7 +11,8 @@
 
 FILES= a.out.h ar.h asr.h assert.h bitstring.h blf.h bsd_auth.h \
complex.h cpio.h ctype.h curses.h db.h dirent.h disktab.h \
-   dlfcn.h elf_abi.h err.h errno.h fenv.h float.h fnmatch.h fstab.h fts.h \
+   dlfcn.h elf.h elf_abi.h err.h errno.h fenv.h float.h fnmatch.h fstab.h \
+   fts.h \
ftw.h getopt.h glob.h grp.h icdb.h ieeefp.h ifaddrs.h inttypes.h \
iso646.h kvm.h langinfo.h libgen.h limits.h link.h link_elf.h \
locale.h login_cap.h math.h md5.h memory.h ndbm.h netdb.h netgroup.h \
diff --git a/include/elf.h b/include/elf.h
new file mode 100644
index 000..c1bc247cc3c
--- /dev/null
+++ b/include/elf.h
@@ -0,0 +1,33 @@
+/* $OpenBSD: elf.h,v 1.4 1996/05/22 07:46:22 etheisen Exp $*/
+/*
+ * Copyright (c) 1996 Erik Theisen
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *derived from this software without specific prior written permission
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef _ELF_H_
+#define _ELF_H_
+
+#include 
+
+#endif /* _ELF_H_ */



Re: magic.5: Add missing types

2017-07-03 Thread Nicholas Marriott
Hi

On Thu, Jun 29, 2017 at 09:29:57PM +0200, Klemens Nanni wrote:
> While reading file(1)'s code in #openbsd-daily mulander noted that the
> 'name' and 'use' types were missing from magic(5).
> 
> I'm not entirely sure yet whether this is complete, so here's what I
> did:
> 
> magic(5) provided by devel/magic documents version 5.31 while base's
> magic(5) is at 4.24. Here are the types found in 5.31 but not ours as
> well as those of the missing ones that are actually implemented but
> undocumented as of now:
> 
>   $ grep -i "TYPE_($(grep -F 'It Dv' $(man -w magic) |
>   cut -d' ' -f3 | sort | uniq -u | paste -sd\| - |
>   tee /dev/stderr))" magic.h
>   beid3|beqwdate|clear|indirect|leid3|leqwdate|name|qwdate|use
>   MAGIC_TYPE_CLEAR,
>   MAGIC_TYPE_NAME,
>   MAGIC_TYPE_USE,
> 
> What about the current version being 4.21? We're clearly ahead of this,
> it seems magic(5) wasn't updated when nicm@ reimplemented things.
> 
> This patch documents the respective types.

Did you copy this text from upstream or write it?

Some comments inline.

> Feedback/OK?
> 
> Index: magic.5
> ===
> RCS file: /cvs/src/usr.bin/file/magic.5,v
> retrieving revision 1.17
> diff -u -p -r1.17 magic.5
> --- magic.5   24 Apr 2016 07:02:07 -  1.17
> +++ magic.5   29 Jun 2017 17:41:56 -
> @@ -218,6 +218,31 @@ This is intended to be used with the tes
> .Em x
> (which is always true) and a message that is to be used if there are
> no other matches.
> +.It Dv clear
> +This test is always true and clears the match flag for that continuation
> +level.

I would just say "that level" or "the test's level" not "that
continuation level" because there is no previous mention of
"continuation".

> +It is intended to be used with the default test.
> +.It Dv name
> +Define a
> +.Dq named

I don't think quotation marks are necessary here.

> +magic instance that can be called from another
> +.Dv use
> +magic entry, like a subroutine call.
> +Named instance direct magic offsets are relative to the offset of the
> +previous matched entry, but indirect offsets are relative to the
> +beginning of the file as usual.
> +Named magic entries always match.
> +.It Dv use
> +Recursively call the named magic starting from the current offset.
> +If the name of the referenced begins with a

"the referenced instance" would be better than "the referenced".

> +.Dv ^
> +then the endianness of the magic is switched; if the magic mentioned
> +.Dv leshort
> +for example,
> +it is treated as
> +.Dv beshort
> +and vice versa.
> +This is useful to avoid duplicating the rules for different endianness.
> .El
> .Pp
> Each top-level magic pattern (see below for an explanation of levels)



Re: sblock() & solock() ordering

2017-07-03 Thread Alexander Bluhm
On Mon, Jul 03, 2017 at 11:42:19AM +0200, Martin Pieuchot wrote:
> Updated diff that fixes some issues reported by visa@:
> 
>   - prevents relocking the netlock in the 'restart' case.
>   - always call solock() after sbunlock() in sosplice().
> 
> Alexander is there an easy way to trigger the 'restart' case in the
> regression tests?  I ran the relayd regress but I'm not sure which
> kernel code it exercises.

See /usr/src/regress/sys/kern/sosplice, it runs all tests with and
without socket splicing.  Goal was to see that behavior is identical.
It tries a bunch of timings and dataflow variants with user land
sleep, send and receive syscalls.  I triggered many goto restart
in soreceive() and a few in sosend().

> Anyway, ok?

OK bluhm@

> 
> Index: sys/socketvar.h
> ===
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.70
> diff -u -p -r1.70 socketvar.h
> --- sys/socketvar.h   26 Jun 2017 09:32:32 -  1.70
> +++ sys/socketvar.h   3 Jul 2017 08:38:14 -
> @@ -239,7 +239,7 @@ struct rwlock;
>   * Unless SB_NOINTR is set on sockbuf, sleep is interruptible.
>   * Returns error without lock if sleep is interrupted.
>   */
> -int sblock(struct sockbuf *, int, struct rwlock *);
> +int sblock(struct socket *, struct sockbuf *, int);
>  
>  /* release lock on sockbuf sb */
>  void sbunlock(struct sockbuf *);
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.191
> diff -u -p -r1.191 uipc_socket.c
> --- kern/uipc_socket.c3 Jul 2017 08:29:24 -   1.191
> +++ kern/uipc_socket.c3 Jul 2017 08:41:24 -
> @@ -418,14 +418,14 @@ sosend(struct socket *so, struct mbuf *a
>   (sizeof(struct fdpass) / sizeof(int)));
>   }
>  
> -#define  snderr(errno)   { error = errno; sounlock(s); goto release; }
> +#define  snderr(errno)   { error = errno; goto release; }
>  
> + s = solock(so);
>  restart:
> - if ((error = sblock(>so_snd, SBLOCKWAIT(flags), NULL)) != 0)
> + if ((error = sblock(so, >so_snd, SBLOCKWAIT(flags))) != 0)
>   goto out;
>   so->so_state |= SS_ISSENDING;
>   do {
> - s = solock(so);
>   if (so->so_state & SS_CANTSENDMORE)
>   snderr(EPIPE);
>   if (so->so_error) {
> @@ -455,12 +455,10 @@ restart:
>   sbunlock(>so_snd);
>   error = sbwait(so, >so_snd);
>   so->so_state &= ~SS_ISSENDING;
> - sounlock(s);
>   if (error)
>   goto out;
>   goto restart;
>   }
> - sounlock(s);
>   space -= clen;
>   do {
>   if (uio == NULL) {
> @@ -471,8 +469,9 @@ restart:
>   if (flags & MSG_EOR)
>   top->m_flags |= M_EOR;
>   } else {
> - error = m_getuio(, atomic,
> - space, uio);
> + sounlock(s);
> + error = m_getuio(, atomic, space, uio);
> + s = solock(so);
>   if (error)
>   goto release;
>   space -= top->m_pkthdr.len;
> @@ -480,7 +479,6 @@ restart:
>   if (flags & MSG_EOR)
>   top->m_flags |= M_EOR;
>   }
> - s = solock(so);
>   if (resid == 0)
>   so->so_state &= ~SS_ISSENDING;
>   if (top && so->so_options & SO_ZEROIZE)
> @@ -488,7 +486,6 @@ restart:
>   error = (*so->so_proto->pr_usrreq)(so,
>   (flags & MSG_OOB) ? PRU_SENDOOB : PRU_SEND,
>   top, addr, control, curproc);
> - sounlock(s);
>   clen = 0;
>   control = NULL;
>   top = NULL;
> @@ -501,6 +498,7 @@ release:
>   so->so_state &= ~SS_ISSENDING;
>   sbunlock(>so_snd);
>  out:
> + sounlock(s);
>   m_freem(top);
>   m_freem(control);
>   return (error);
> @@ -670,9 +668,11 @@ bad:
>   *mp = NULL;
>  
>  restart:
> - if ((error = sblock(>so_rcv, SBLOCKWAIT(flags), NULL)) != 0)
> - return (error);
>   s = solock(so);
> + if ((error = sblock(so, >so_rcv, SBLOCKWAIT(flags))) != 0) {
> + sounlock(s);
> + return (error);
> + }
>  
>   m = so->so_rcv.sb_mb;
>  #ifdef SOCKET_SPLICE
> @@ -1040,13 +1040,10 @@ sorflush(struct socket *so)
>  {
>   struct sockbuf *sb = >so_rcv;
>   struct 

Re: ifstated readability diff

2017-07-03 Thread Rob Pierce
On Sun, Jul 02, 2017 at 11:50:56PM -0400, Rob Pierce wrote:
> Remove obvious clear_config() comments and misleading state_change() comments.
> 
> Also relocate do_action() calls for the init block from change_state() to
> occur with the corresponding do_action() calls for the body block within
> the calling function for improved readability.
> 
> No functional change.
> 
> Rob

That diff was broken. This one is correct.

Rob

Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.48
diff -u -p -r1.48 ifstated.c
--- ifstated.c  2 Jul 2017 15:28:26 -   1.48
+++ ifstated.c  3 Jul 2017 13:42:13 -
@@ -217,8 +217,10 @@ load_config(void)
conf->curstate->entered = time(NULL);
conf->nextstate = conf->curstate;
conf->curstate = NULL;
-   while (state_change())
+   while (state_change()) {
+   do_action(conf->curstate->init);
do_action(conf->curstate->body);
+   }
}
return (0);
 }
@@ -533,14 +535,13 @@ eval_state(struct ifsd_state *state)
if (external == NULL || external->lastexec >= state->entered ||
external->lastexec == 0) {
do_action(state->body);
-   while (state_change())
+   while (state_change()) {
+   do_action(conf->curstate->init);
do_action(conf->curstate->body);
+   }
}
 }
 
-/*
- *If a previous action included a state change, process it.
- */
 int
 state_change(void)
 {
@@ -556,7 +557,6 @@ state_change(void)
conf->curstate->entered = time(NULL);
external_evtimer_setup(conf->curstate, IFSD_EVTIMER_ADD);
adjust_external_expressions(conf->curstate);
-   do_action(conf->curstate->init);
return (1);
}
return (0);
@@ -627,9 +627,6 @@ fetch_state(void)
close(sock);
 }
 
-/*
- * Clear the config.
- */
 void
 clear_config(struct ifsd_config *oconf)
 {



Re: fo_ioctl & solock

2017-07-03 Thread Alexander Bluhm
On Mon, Jul 03, 2017 at 02:48:49PM +0200, Martin Pieuchot wrote:
> soo_ioctl() will need to grab the socket lock since it modifies its
> states.  Sadly this function is sometimes called from socket-only
> syscalls which already held the corresponding socket lock.
> 
> So the diff below simply set/remove SS_NBIO directly in places where
> we are dealing with sockets and already have the lock.
> 
> ok?

OK bluhm@

> 
> Index: kern/uipc_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> retrieving revision 1.151
> diff -u -p -r1.151 uipc_syscalls.c
> --- kern/uipc_syscalls.c  27 Mar 2017 11:45:49 -  1.151
> +++ kern/uipc_syscalls.c  3 Jul 2017 12:37:03 -
> @@ -110,10 +110,10 @@ sys_socket(struct proc *p, void *v, regi
>   closef(fp, p);
>   fdpunlock(fdp);
>   } else {
> - fp->f_data = so;
>   if (type & SOCK_NONBLOCK)
> - (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t), p);
> + so->so_state |= SS_NBIO;
>   so->so_state |= ss;
> + fp->f_data = so;
>   FILE_SET_MATURE(fp, p);
>   *retval = fd;
>   }
> @@ -332,12 +332,15 @@ doaccept(struct proc *p, int sock, struc
>   fp->f_type = DTYPE_SOCKET;
>   fp->f_flag = FREAD | FWRITE | nflag;
>   fp->f_ops = 
> - fp->f_data = so;
>   error = soaccept(so, nam);
>   if (!error && name != NULL)
>   error = copyaddrout(p, nam, name, namelen, anamelen);
>   if (!error) {
> - (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t), p);
> + if (nflag & FNONBLOCK)
> + so->so_state |= SS_NBIO;
> + else
> + so->so_state &= ~SS_NBIO;
> + fp->f_data = so;
>   FILE_SET_MATURE(fp, p);
>   *retval = tmpfd;
>   }



Re: so_state & solock

2017-07-03 Thread Alexander Bluhm
On Mon, Jul 03, 2017 at 02:42:15PM +0200, Martin Pieuchot wrote:
> I'd like to assert that the socket lock is held when modifying
> `so_sate'.
> 
> ok?

OK bluhm@

> 
> Index: kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 uipc_socket2.c
> --- kern/uipc_socket2.c   27 Jun 2017 12:02:43 -  1.80
> +++ kern/uipc_socket2.c   3 Jul 2017 12:35:46 -
> @@ -89,7 +89,7 @@ int sbsleep(struct sockbuf *, struct rwl
>  void
>  soisconnecting(struct socket *so)
>  {
> -
> + soassertlocked(so);
>   so->so_state &= ~(SS_ISCONNECTED|SS_ISDISCONNECTING);
>   so->so_state |= SS_ISCONNECTING;
>  }
> @@ -99,6 +99,7 @@ soisconnected(struct socket *so)
>  {
>   struct socket *head = so->so_head;
>  
> + soassertlocked(so);
>   so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING);
>   so->so_state |= SS_ISCONNECTED;
>   if (head && soqremque(so, 0)) {
> @@ -115,7 +116,7 @@ soisconnected(struct socket *so)
>  void
>  soisdisconnecting(struct socket *so)
>  {
> -
> + soassertlocked(so);
>   so->so_state &= ~SS_ISCONNECTING;
>   so->so_state |= (SS_ISDISCONNECTING|SS_CANTRCVMORE|SS_CANTSENDMORE);
>   wakeup(>so_timeo);
> @@ -126,7 +127,7 @@ soisdisconnecting(struct socket *so)
>  void
>  soisdisconnected(struct socket *so)
>  {
> -
> + soassertlocked(so);
>   so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
>   so->so_state |= (SS_CANTRCVMORE|SS_CANTSENDMORE|SS_ISDISCONNECTED);
>   wakeup(>so_timeo);
> @@ -259,7 +260,7 @@ soqremque(struct socket *so, int q)
>  void
>  socantsendmore(struct socket *so)
>  {
> -
> + soassertlocked(so);
>   so->so_state |= SS_CANTSENDMORE;
>   sowwakeup(so);
>  }
> @@ -267,7 +268,7 @@ socantsendmore(struct socket *so)
>  void
>  socantrcvmore(struct socket *so)
>  {
> -
> + soassertlocked(so);
>   so->so_state |= SS_CANTRCVMORE;
>   sorwakeup(so);
>  }



Re: so_qlen & solock

2017-07-03 Thread Alexander Bluhm
On Mon, Jul 03, 2017 at 02:41:15PM +0200, Martin Pieuchot wrote:
> I'd like to assert the socket lock is held when `so_qlen' is modified
> or when it is accessed as part of a sequence that needs atomicity.
> 
> ok?

OK bluhm@

> 
> Index: kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 uipc_socket2.c
> --- kern/uipc_socket2.c   27 Jun 2017 12:02:43 -  1.80
> +++ kern/uipc_socket2.c   3 Jul 2017 12:34:18 -
> @@ -208,6 +208,7 @@ sonewconn(struct socket *head, int conns
>  void
>  soqinsque(struct socket *head, struct socket *so, int q)
>  {
> + soassertlocked(head);
>  
>  #ifdef DIAGNOSTIC
>   if (so->so_onq != NULL)
> @@ -228,9 +229,10 @@ soqinsque(struct socket *head, struct so
>  int
>  soqremque(struct socket *so, int q)
>  {
> - struct socket *head;
> + struct socket *head = so->so_head;
> +
> + soassertlocked(head);
>  
> - head = so->so_head;
>   if (q == 0) {
>   if (so->so_onq != >so_q0)
>   return (0);
> Index: sys/socketvar.h
> ===
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.70
> diff -u -p -r1.70 socketvar.h
> --- sys/socketvar.h   26 Jun 2017 09:32:32 -  1.70
> +++ sys/socketvar.h   3 Jul 2017 12:34:18 -
> @@ -199,11 +199,15 @@ sbspace(struct socket *so, struct sockbu
>  ((so)->so_state & SS_ISSENDING)
>  
>  /* can we read something from so? */
> -#define  soreadable(so)  \
> -(!isspliced(so) && \
> -((so)->so_rcv.sb_cc >= (so)->so_rcv.sb_lowat || \
> -((so)->so_state & SS_CANTRCVMORE) || \
> -(so)->so_qlen || (so)->so_error))
> +static inline int
> +soreadable(struct socket *so)
> +{
> + soassertlocked(so);
> + if (isspliced(so))
> + return 0;
> + return (so->so_state & SS_CANTRCVMORE) || so->so_qlen || so->so_error ||
> + so->so_rcv.sb_cc >= so->so_rcv.sb_lowat;
> +}
>  
>  /* can we write something to so? */
>  #define  sowriteable(so) \



Re: isakmpd(8) use-after-free

2017-07-03 Thread Martin Pieuchot
On 03/07/17(Mon) 11:18, Martin Pieuchot wrote:
> On 08/06/17(Thu) 15:23, Martin Pieuchot wrote:
> > Michał Koc reported a crash on misc@, turns out it's a use-after-free:
> > http://marc.info/?l=openbsd-misc=149597472223216=2
> > 
> > The trace indicates that argument given to pf_key_v2_stayalive() is no
> > longer valid:
> > 
> >   #0  conf_get_str (section=0xa8735b03f80 '   >   0xa8735b04000 out of bounds>, tag=0xa8459272809 "Phase") at 
> > /usr/src/sbin/isakmpd/conf.c:94
> >   #1  0x0a84590293b4 in pf_key_v2_remove_conf (section=0xa8735b03f80 ' 
> >  ) at 
> > /usr/src/sbin/isakmpd/pf_key_v2.c:1905
> >   #2  0x0a845902956a in pf_key_v2_stayalive (exchange=0xa86a6f44200, 
> > vconn=0xa8735b03f80, fail=1) at /usr/src/sbin/isakmpd/pf_key_v2.c:2131
> > 
> > 
> > In r1.58 of pf_key_v2.c angelos@ move the argument given to
> > pf_key_v2_connection_check(), the one used after free, from
> > the stack to the heap:
> > 
> >Dynamically allocate conn, as this is given to the exchange; cleanup 
> >
> >conf space on failure to establish dynamic SA. ok niklas@
> > 
> > I don't understand the whole magic of function pointers in exchange.c
> > but what's interesting is that in his diff he stopped dereferencing
> > 'exchange->name'.
> > 
> > But in pf_key_v2_connection_check() the 'conn' argument is passed as
> > 'name' and as 'arg'...  So the diff below fixes Michał's problem.  I'd
> > appreciate if more people could test it and check if isakmpd(8) do not
> > leaking more memory than it already does.
> > 
> > Note that this diff do not fix the 'conn' leak introduced in the above
> > mentioned commit when a connection exist and exchange_establish() is
> > not called.
> 
> It turns out that the problem comes from connection_checker().  This
> function did not get the fix applied by angelos@ in r1.58 of pf_key_v2.c.
> 
> Since 'conn' is given to exchange_establish() it must be allocated
> dynamically.
> 
> Diff below also prevents a use-after-free in connection_record_passive()
> and plugs memory leaks in pf_key_v2_connection_check().

hshoexer@ found that my new diff generalized a memory leak present in
pf_key_v2_acquire(). 

Updated diff below fixes that by freeing the memory in pf_key_v2_stayalive().

Comments, ok?

Index: connection.c
===
RCS file: /cvs/src/sbin/isakmpd/connection.c,v
retrieving revision 1.37
diff -u -p -r1.37 connection.c
--- connection.c22 Jan 2014 03:09:31 -  1.37
+++ connection.c3 Jul 2017 09:03:53 -
@@ -146,6 +146,7 @@ connection_checker(void *vconn)
 {
struct timeval  now;
struct connection *conn = vconn;
+   char *name;
 
gettimeofday(, 0);
now.tv_sec += conf_get_num("General", "check-interval",
@@ -153,9 +154,16 @@ connection_checker(void *vconn)
conn->ev = timer_add_event("connection_checker",
connection_checker, conn, );
if (!conn->ev)
-   log_print("connection_checker: could not add timer event");
-   if (!ui_daemon_passive)
-   pf_key_v2_connection_check(conn->name);
+   log_print("%s: could not add timer event", __func__);
+   if (ui_daemon_passive)
+   return;
+
+   name = strdup(conn->name);
+   if (!name) {
+   log_print("%s: strdup (\"%s\") failed", __func__, conn->name);
+   return;
+   }
+   pf_key_v2_connection_check(name);
 }
 
 /* Find the connection named NAME.  */
Index: pf_key_v2.c
===
RCS file: /cvs/src/sbin/isakmpd/pf_key_v2.c,v
retrieving revision 1.198
diff -u -p -r1.198 pf_key_v2.c
--- pf_key_v2.c 28 Feb 2017 16:46:27 -  1.198
+++ pf_key_v2.c 3 Jul 2017 12:53:29 -
@@ -2131,6 +2131,7 @@ pf_key_v2_stayalive(struct exchange *exc
pf_key_v2_remove_conf(conn);
pf_key_v2_remove_conf(conn);
}
+   free(conn);
 }
 
 /* Check if a connection CONN exists, otherwise establish it.  */
@@ -2141,9 +2142,11 @@ pf_key_v2_connection_check(char *conn)
LOG_DBG((LOG_SYSDEP, 70,
"pf_key_v2_connection_check: SA for %s missing", conn));
exchange_establish(conn, pf_key_v2_stayalive, conn, 0);
-   } else
+   } else {
LOG_DBG((LOG_SYSDEP, 70, "pf_key_v2_connection_check: "
"SA for %s exists", conn));
+   free(conn);
+   }
 }
 
 /* Handle a PF_KEY lifetime expiration message PMSG.  */
@@ -3144,8 +3147,8 @@ pf_key_v2_acquire(struct pf_key_v2_msg *
conf_end(af, 1);
 
/* Let's rock 'n roll. */
-   pf_key_v2_connection_check(conn);
connection_record_passive(conn);
+   pf_key_v2_connection_check(conn);
conn = 0;
 
/* Fall-through to cleanup. */



Re: CVS: cvs.openbsd.org: src

2017-07-03 Thread Martijn van Duren
On 07/01/17 18:14, Mark Kettenis wrote:
> CVSROOT:  /cvs
> Module name:  src
> Changes by:   kette...@cvs.openbsd.org2017/07/01 10:14:10
> 
> Modified files:
>   sys/dev/pci/drm: drm_irq.c drm_linux.c drm_linux.h 
>drm_linux_list.h drm_mm.c drm_mm.h drm_mode.h 
>drm_modes.c drm_rect.c drm_rect.h 
>drm_vma_manager.c files.drm i915_drm.h 
>i915_pciids.h linux_hdmi.h 
>   sys/dev/pci/drm/i915: dvo.h dvo_ch7017.c dvo_ch7xxx.c dvo_ivch.c 
> dvo_ns2501.c dvo_sil164.c dvo_tfp410.c 
> i915_dma.c i915_drv.c i915_drv.h 
> i915_gem.c i915_gem_context.c 
> i915_gem_evict.c i915_gem_execbuffer.c 
> i915_gem_gtt.c i915_gem_stolen.c 
> i915_gem_tiling.c i915_gpu_error.c 
> i915_irq.c i915_reg.h i915_suspend.c 
> i915_trace.h intel_bios.c intel_bios.h 
> intel_crt.c intel_ddi.c intel_display.c 
> intel_dp.c intel_drv.h intel_dvo.c 
> intel_fbdev.c intel_hdmi.c intel_i2c.c 
> intel_lvds.c intel_modes.c 
> intel_opregion.c intel_overlay.c 
> intel_panel.c intel_pm.c 
> intel_ringbuffer.c intel_ringbuffer.h 
> intel_sdvo.c intel_sdvo_regs.h 
> intel_sideband.c intel_sprite.c intel_tv.c 
> intel_uncore.c 
>   sys/dev/pci/drm/radeon: atombios_crtc.c atombios_dp.c 
>   atombios_i2c.c nid.h r100.c radeon.h 
>   radeon_connectors.c radeon_device.c 
>   radeon_display.c radeon_fb.c 
>   radeon_i2c.c radeon_irq_kms.c 
>   radeon_kms.c radeon_legacy_crtc.c 
>   radeon_legacy_encoders.c radeon_mode.h 
>   radeon_pm.c 
>   sys/dev/pci/drm/ttm: ttm_bo_manager.c 
> Added files:
>   sys/dev/pci/drm: drm_linux_atomic.h drm_mipi_dsi.h drm_modes.h 
>drm_modeset_lock.c drm_modeset_lock.h 
>drm_panel.c drm_panel.h drm_plane_helper.c 
>drm_plane_helper.h drm_probe_helper.c 
>linux_list_sort.c linux_types.h 
>linux_ww_mutex.h 
>   sys/dev/pci/drm/i915: i915_cmd_parser.c i915_gem_batch_pool.c 
> i915_gem_batch_pool.h i915_gem_fence.c 
> i915_gem_gtt.h i915_gem_render_state.c 
> i915_gem_render_state.h i915_gem_userptr.c 
> i915_guc_reg.h i915_guc_submission.c 
> i915_params.c i915_vgpu.c i915_vgpu.h 
> intel_atomic.c intel_atomic_plane.c 
> intel_audio.c intel_csr.c intel_dp_mst.c 
> intel_dsi.c intel_dsi.h 
> intel_dsi_panel_vbt.c intel_dsi_pll.c 
> intel_fbc.c intel_fifo_underrun.c 
> intel_frontbuffer.c intel_gtt.c 
> intel_guc.h intel_guc_fwif.h 
> intel_guc_loader.c intel_hotplug.c 
> intel_lrc.c intel_lrc.h intel_mocs.c 
> intel_mocs.h intel_psr.c 
> intel_renderstate.h 
> intel_renderstate_gen6.c 
> intel_renderstate_gen7.c 
> intel_renderstate_gen8.c 
> intel_renderstate_gen9.c 
> intel_runtime_pm.c 
>   sys/dev/pci/drm/radeon: radeon_dp_auxch.c 
> 
> Log message:
> Update inteldrm(4) to code based on Linux 4.4.70.  This brings us support for
> Skylake and Cherryview and better support for Broadwell and Valleyview.  Also
> adds MST support.  Some tweaks to the TTM code and radeondrm(4) to keep it
> working with the updated generic DRM code needed for inteldrm(4).
> 
> Tested by many.
> 
This change *STILL* breaks my $DAYJOB machine.

dmesg with DRMDEBUG enabled

OpenBSD 6.1-current (GENERIC.MP) #21: Mon Jul  3 08:14:45 CEST 2017

mart...@martijn.office.rootnet.nl:/home/martijn/src/OpenBSD/sys/arch/amd64/compile/GENERIC.MP
RTC BIOS diagnostic error d1
real mem = 4253237248 (4056MB)
avail mem = 4118540288 (3927MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0x8ed34000 (61 entries)
bios0: vendor Apple Inc. version 

fo_ioctl & solock

2017-07-03 Thread Martin Pieuchot
soo_ioctl() will need to grab the socket lock since it modifies its
states.  Sadly this function is sometimes called from socket-only
syscalls which already held the corresponding socket lock.

So the diff below simply set/remove SS_NBIO directly in places where
we are dealing with sockets and already have the lock.

ok?

Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.151
diff -u -p -r1.151 uipc_syscalls.c
--- kern/uipc_syscalls.c27 Mar 2017 11:45:49 -  1.151
+++ kern/uipc_syscalls.c3 Jul 2017 12:37:03 -
@@ -110,10 +110,10 @@ sys_socket(struct proc *p, void *v, regi
closef(fp, p);
fdpunlock(fdp);
} else {
-   fp->f_data = so;
if (type & SOCK_NONBLOCK)
-   (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t), p);
+   so->so_state |= SS_NBIO;
so->so_state |= ss;
+   fp->f_data = so;
FILE_SET_MATURE(fp, p);
*retval = fd;
}
@@ -332,12 +332,15 @@ doaccept(struct proc *p, int sock, struc
fp->f_type = DTYPE_SOCKET;
fp->f_flag = FREAD | FWRITE | nflag;
fp->f_ops = 
-   fp->f_data = so;
error = soaccept(so, nam);
if (!error && name != NULL)
error = copyaddrout(p, nam, name, namelen, anamelen);
if (!error) {
-   (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t), p);
+   if (nflag & FNONBLOCK)
+   so->so_state |= SS_NBIO;
+   else
+   so->so_state &= ~SS_NBIO;
+   fp->f_data = so;
FILE_SET_MATURE(fp, p);
*retval = tmpfd;
}



so_state & solock

2017-07-03 Thread Martin Pieuchot
I'd like to assert that the socket lock is held when modifying
`so_sate'.

ok?

Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.80
diff -u -p -r1.80 uipc_socket2.c
--- kern/uipc_socket2.c 27 Jun 2017 12:02:43 -  1.80
+++ kern/uipc_socket2.c 3 Jul 2017 12:35:46 -
@@ -89,7 +89,7 @@ int sbsleep(struct sockbuf *, struct rwl
 void
 soisconnecting(struct socket *so)
 {
-
+   soassertlocked(so);
so->so_state &= ~(SS_ISCONNECTED|SS_ISDISCONNECTING);
so->so_state |= SS_ISCONNECTING;
 }
@@ -99,6 +99,7 @@ soisconnected(struct socket *so)
 {
struct socket *head = so->so_head;
 
+   soassertlocked(so);
so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING);
so->so_state |= SS_ISCONNECTED;
if (head && soqremque(so, 0)) {
@@ -115,7 +116,7 @@ soisconnected(struct socket *so)
 void
 soisdisconnecting(struct socket *so)
 {
-
+   soassertlocked(so);
so->so_state &= ~SS_ISCONNECTING;
so->so_state |= (SS_ISDISCONNECTING|SS_CANTRCVMORE|SS_CANTSENDMORE);
wakeup(>so_timeo);
@@ -126,7 +127,7 @@ soisdisconnecting(struct socket *so)
 void
 soisdisconnected(struct socket *so)
 {
-
+   soassertlocked(so);
so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
so->so_state |= (SS_CANTRCVMORE|SS_CANTSENDMORE|SS_ISDISCONNECTED);
wakeup(>so_timeo);
@@ -259,7 +260,7 @@ soqremque(struct socket *so, int q)
 void
 socantsendmore(struct socket *so)
 {
-
+   soassertlocked(so);
so->so_state |= SS_CANTSENDMORE;
sowwakeup(so);
 }
@@ -267,7 +268,7 @@ socantsendmore(struct socket *so)
 void
 socantrcvmore(struct socket *so)
 {
-
+   soassertlocked(so);
so->so_state |= SS_CANTRCVMORE;
sorwakeup(so);
 }



so_qlen & solock

2017-07-03 Thread Martin Pieuchot
I'd like to assert the socket lock is held when `so_qlen' is modified
or when it is accessed as part of a sequence that needs atomicity.

ok?

Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.80
diff -u -p -r1.80 uipc_socket2.c
--- kern/uipc_socket2.c 27 Jun 2017 12:02:43 -  1.80
+++ kern/uipc_socket2.c 3 Jul 2017 12:34:18 -
@@ -208,6 +208,7 @@ sonewconn(struct socket *head, int conns
 void
 soqinsque(struct socket *head, struct socket *so, int q)
 {
+   soassertlocked(head);
 
 #ifdef DIAGNOSTIC
if (so->so_onq != NULL)
@@ -228,9 +229,10 @@ soqinsque(struct socket *head, struct so
 int
 soqremque(struct socket *so, int q)
 {
-   struct socket *head;
+   struct socket *head = so->so_head;
+
+   soassertlocked(head);
 
-   head = so->so_head;
if (q == 0) {
if (so->so_onq != >so_q0)
return (0);
Index: sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.70
diff -u -p -r1.70 socketvar.h
--- sys/socketvar.h 26 Jun 2017 09:32:32 -  1.70
+++ sys/socketvar.h 3 Jul 2017 12:34:18 -
@@ -199,11 +199,15 @@ sbspace(struct socket *so, struct sockbu
 ((so)->so_state & SS_ISSENDING)
 
 /* can we read something from so? */
-#definesoreadable(so)  \
-(!isspliced(so) && \
-((so)->so_rcv.sb_cc >= (so)->so_rcv.sb_lowat || \
-((so)->so_state & SS_CANTRCVMORE) || \
-(so)->so_qlen || (so)->so_error))
+static inline int
+soreadable(struct socket *so)
+{
+   soassertlocked(so);
+   if (isspliced(so))
+   return 0;
+   return (so->so_state & SS_CANTRCVMORE) || so->so_qlen || so->so_error ||
+   so->so_rcv.sb_cc >= so->so_rcv.sb_lowat;
+}
 
 /* can we write something to so? */
 #definesowriteable(so) \



Re: netstat(1) print PID for sockets.

2017-07-03 Thread Alexander Bluhm
On Sat, Jul 01, 2017 at 08:50:36PM +0200, Sebastian Benoit wrote:
> @@ -149,7 +149,7 @@ protopr(kvm_t *kvmd, u_long pcbaddr, u_int tableid, int 
> proto)
>   struct kinfo_file *kf;
>   int i, fcnt;
>  
> - kf = kvm_getfiles(kvmd, KERN_FILE_BYFILE, DTYPE_SOCKET,
> + kf = kvm_getfiles(kvmd, KERN_FILE_BYPID, -1,
>   sizeof(*kf), );
>   if (kf == NULL) {
>   printf("Out of memory (file table).\n");

This will not display TCP sockets that have been closed by the
process.  It is important to see them as they still allocate kernel
resources.  The KERN_FILE_BYFILE sysctl loops over the PCB tables
in the kernel to make this possible.

bluhm



Re: netstat(1) show only listening sockets

2017-07-03 Thread Alexander Bluhm
On Sat, Jul 01, 2017 at 04:44:14PM +0200, Sebastian Benoit wrote:
> This makes netstat show only listening sockets for tcp sockets
> when invoked as netstat -l.

> @@ -294,9 +294,14 @@ netdomainpr(struct kinfo_file *kf, int proto)
>   }
>  
>   /* filter listening sockets out unless -a is set */
> - if (!aflag && istcp && kf->t_state <= TCPS_LISTEN)
> + if (!(aflag||lflag) && istcp && kf->t_state <= TCPS_LISTEN)
>   return;
> - else if (!aflag && isany)
> + else if (!(aflag||lflag) && isany)
> + return;
> +
> + /* when -l is set, show only listening sockets */
> + if (!aflag && lflag && kf->so_type == SOCK_STREAM &&
> + kf->t_state != TCPS_LISTEN)
>   return;
>  

Only sockets with protocol tcp have t_state.  This check sould be
"istcp" like a few lines above instead of "kf->so_type == SOCK_STREAM".

otherwise OK bluhm@



Re: netstat(1) show only listening sockets

2017-07-03 Thread Craig Skinner
Hi Sebastian,

On Sat, 1 Jul 2017 16:44:14 +0200 Sebastian Benoit wrote:
> This makes netstat show only listening sockets for tcp sockets
> when invoked as netstat -l.
> 
> With it "netstat -l -finet -p tcp" is equivalent to
> "netstat -a -finet | grep LISTEN"

This shows listening UDP ports too:

$ netstat -a -f inet | fgrep '*.*'

Or limit to only 'Proto' & 'Local Address' fields:

$ netstat -a -f inet | awk '/\*.\*/ { print $1"\t"$4 }'

Cheers,
-- 
Craig Skinner | http://linkd.in/yGqkv7



NET_LOCK() w/o SPL

2017-07-03 Thread Martin Pieuchot
All network processing contexts, with the exception of hardware
interrupt handlers, are now process contexts.  That means the SPL
protection is no longer needed inside the NET_LOCK().

So the diff below removes the splsofnet()/splx() dance from the
NET_LOCK().  I'm not changing the NET_LOCK() macro in the same
diff in case we find some unexpected bugs and need to revert this
change.

ok?

Index: sys/systm.h
===
RCS file: /cvs/src/sys/sys/systm.h,v
retrieving revision 1.131
diff -u -p -r1.131 systm.h
--- sys/systm.h 29 May 2017 12:12:35 -  1.131
+++ sys/systm.h 3 Jul 2017 09:54:27 -
@@ -299,12 +299,12 @@ extern struct rwlock netlock;
 #defineNET_LOCK(s) 
\
 do {   \
rw_enter_write();   \
-   s = splsoftnet();   \
+   (void)s;\
 } while (0)
 
 #defineNET_UNLOCK(s)   
\
 do {   \
-   splx(s);\
+   (void)s;\
rw_exit_write();\
 } while (0)
 
@@ -312,7 +312,6 @@ do {
\
 do {   \
if (rw_status() != RW_WRITE)\
splassert_fail(RW_WRITE, rw_status(), __func__);\
-   splsoftassert(IPL_SOFTNET); \
 } while (0)
 
 #defineNET_ASSERT_UNLOCKED()   
\
Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.80
diff -u -p -r1.80 uipc_socket2.c
--- kern/uipc_socket2.c 27 Jun 2017 12:02:43 -  1.80
+++ kern/uipc_socket2.c 3 Jul 2017 10:13:18 -
@@ -279,9 +277,10 @@ solock(struct socket *so)
 
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
(so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
-   (so->so_proto->pr_domain->dom_family != PF_KEY))
+   (so->so_proto->pr_domain->dom_family != PF_KEY)) {
NET_LOCK(s);
-   else
+   s = IPL_SOFTNET; /* arbitrary value */
+   } else
s = -42;
 
return (s);



Re: sblock() & solock() ordering

2017-07-03 Thread Martin Pieuchot
On 26/06/17(Mon) 16:15, Martin Pieuchot wrote:
> I'd like to enforce the following "lock" ordering: always hold the
> socket lock when calling sblock().
> 
> This would allow me to protect `so_state' in sosend() when setting the
> SS_ISSENDING bit.
> 
> Diff below implements that.  It also gets rid of sbsleep() and uses
> sosleep() instead.

Updated diff that fixes some issues reported by visa@:

  - prevents relocking the netlock in the 'restart' case.
  - always call solock() after sbunlock() in sosplice().

Alexander is there an easy way to trigger the 'restart' case in the
regression tests?  I ran the relayd regress but I'm not sure which
kernel code it exercises.

Anyway, ok?

Index: sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.70
diff -u -p -r1.70 socketvar.h
--- sys/socketvar.h 26 Jun 2017 09:32:32 -  1.70
+++ sys/socketvar.h 3 Jul 2017 08:38:14 -
@@ -239,7 +239,7 @@ struct rwlock;
  * Unless SB_NOINTR is set on sockbuf, sleep is interruptible.
  * Returns error without lock if sleep is interrupted.
  */
-int sblock(struct sockbuf *, int, struct rwlock *);
+int sblock(struct socket *, struct sockbuf *, int);
 
 /* release lock on sockbuf sb */
 void sbunlock(struct sockbuf *);
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.191
diff -u -p -r1.191 uipc_socket.c
--- kern/uipc_socket.c  3 Jul 2017 08:29:24 -   1.191
+++ kern/uipc_socket.c  3 Jul 2017 08:41:24 -
@@ -418,14 +418,14 @@ sosend(struct socket *so, struct mbuf *a
(sizeof(struct fdpass) / sizeof(int)));
}
 
-#definesnderr(errno)   { error = errno; sounlock(s); goto release; }
+#definesnderr(errno)   { error = errno; goto release; }
 
+   s = solock(so);
 restart:
-   if ((error = sblock(>so_snd, SBLOCKWAIT(flags), NULL)) != 0)
+   if ((error = sblock(so, >so_snd, SBLOCKWAIT(flags))) != 0)
goto out;
so->so_state |= SS_ISSENDING;
do {
-   s = solock(so);
if (so->so_state & SS_CANTSENDMORE)
snderr(EPIPE);
if (so->so_error) {
@@ -455,12 +455,10 @@ restart:
sbunlock(>so_snd);
error = sbwait(so, >so_snd);
so->so_state &= ~SS_ISSENDING;
-   sounlock(s);
if (error)
goto out;
goto restart;
}
-   sounlock(s);
space -= clen;
do {
if (uio == NULL) {
@@ -471,8 +469,9 @@ restart:
if (flags & MSG_EOR)
top->m_flags |= M_EOR;
} else {
-   error = m_getuio(, atomic,
-   space, uio);
+   sounlock(s);
+   error = m_getuio(, atomic, space, uio);
+   s = solock(so);
if (error)
goto release;
space -= top->m_pkthdr.len;
@@ -480,7 +479,6 @@ restart:
if (flags & MSG_EOR)
top->m_flags |= M_EOR;
}
-   s = solock(so);
if (resid == 0)
so->so_state &= ~SS_ISSENDING;
if (top && so->so_options & SO_ZEROIZE)
@@ -488,7 +486,6 @@ restart:
error = (*so->so_proto->pr_usrreq)(so,
(flags & MSG_OOB) ? PRU_SENDOOB : PRU_SEND,
top, addr, control, curproc);
-   sounlock(s);
clen = 0;
control = NULL;
top = NULL;
@@ -501,6 +498,7 @@ release:
so->so_state &= ~SS_ISSENDING;
sbunlock(>so_snd);
 out:
+   sounlock(s);
m_freem(top);
m_freem(control);
return (error);
@@ -670,9 +668,11 @@ bad:
*mp = NULL;
 
 restart:
-   if ((error = sblock(>so_rcv, SBLOCKWAIT(flags), NULL)) != 0)
-   return (error);
s = solock(so);
+   if ((error = sblock(so, >so_rcv, SBLOCKWAIT(flags))) != 0) {
+   sounlock(s);
+   return (error);
+   }
 
m = so->so_rcv.sb_mb;
 #ifdef SOCKET_SPLICE
@@ -1040,13 +1040,10 @@ sorflush(struct socket *so)
 {
struct sockbuf *sb = >so_rcv;
struct protosw *pr = so->so_proto;
-   sa_family_t af = pr->pr_domain->dom_family;
struct socket aso;
 
sb->sb_flags |= SB_NOINTR;
-   

isakmpd NULL dereference

2017-07-03 Thread Martin Pieuchot
Michał Koc reported another isakmpd(8) crash, this time related to a 
NULL dereference:

#0 0x076e6ff12959 in ipsec_sa_check_flow_any (sa=Variable "sa" is not 
available.) at /usr/src/sbin/isakmpd/ipsec.c:275
#1 0x076e6ff1c215 in sa_find (check=0x76e6ff128d0 
, arg=0x7708ab93a00) at /usr/src/sbin/isakmpd/sa.c:132
#2 0x076e6ff152ca in ipsec_delete_spi (sa=0x7708ab93a00, 
proto=0x771456e4a00, incoming=0) at /usr/src/sbin/isakmpd/ipsec.c:1573
#3 0x076e6ff1c5d1 in proto_free (proto=0x771456e4a00) at 
/usr/src/sbin/isakmpd/sa.c:809
#4 0x076e6ff1ceb5 in sa_release (sa=0x7708ab93a00) at 
/usr/src/sbin/isakmpd/sa.c:884
#5 0x076e6ff06a04 in exchange_free_aux (v_exch=Variable "v_exch" is not 
available.) at /usr/src/sbin/isakmpd/exchange.c:1238
#6 0x076e6ff1e8f0 in timer_handle_expirations () at 
/usr/src/sbin/isakmpd/timer.c:76
#7 0x076e6ff15b1b in main (argc=Variable "argc" is not available.) at 
/usr/src/sbin/isakmpd/isakmpd.c:533

In this case sa_find() is called from a timer.  It iterates over all the
`sa' but some of them might be uninitialized.   This can happen if
`finalize_exchange' hasn't yet been called.  But in this case we know,
or at least can't tell if, the SA won't match the flow of another one.
So simply bail.

ok?

Index: ipsec.c
===
RCS file: /cvs/src/sbin/isakmpd/ipsec.c,v
retrieving revision 1.146
diff -u -p -r1.146 ipsec.c
--- ipsec.c 10 Dec 2015 17:27:00 -  1.146
+++ ipsec.c 3 Jul 2017 09:24:39 -
@@ -272,6 +272,15 @@ ipsec_sa_check_flow_any(struct sa *sa, v
isa->dport != isa2->dport)
return 0;
 
+   /*
+* If at least one of the IPsec SAs is incomplete, we're done.
+*/
+   if (isa->src_net == NULL || isa2->src_net == NULL ||
+   isa->dst_net == NULL || isa2->dst_net == NULL ||
+   isa->src_mask == NULL || isa2->src_mask == NULL ||
+   isa->dst_mask == NULL || isa2->dst_mask == NULL)
+   return 0;
+
return isa->src_net->sa_family == isa2->src_net->sa_family &&
memcmp(sockaddr_addrdata(isa->src_net),
sockaddr_addrdata(isa2->src_net),



Re: isakmpd(8) use-after-free

2017-07-03 Thread Martin Pieuchot
On 08/06/17(Thu) 15:23, Martin Pieuchot wrote:
> Michał Koc reported a crash on misc@, turns out it's a use-after-free:
>   http://marc.info/?l=openbsd-misc=149597472223216=2
> 
> The trace indicates that argument given to pf_key_v2_stayalive() is no
> longer valid:
> 
>   #0  conf_get_str (section=0xa8735b03f80 ' 0xa8735b04000 out of bounds>, tag=0xa8459272809 "Phase") at 
> /usr/src/sbin/isakmpd/conf.c:94
>   #1  0x0a84590293b4 in pf_key_v2_remove_conf (section=0xa8735b03f80 ' 
>  ) at 
> /usr/src/sbin/isakmpd/pf_key_v2.c:1905
>   #2  0x0a845902956a in pf_key_v2_stayalive (exchange=0xa86a6f44200, 
> vconn=0xa8735b03f80, fail=1) at /usr/src/sbin/isakmpd/pf_key_v2.c:2131
> 
> 
> In r1.58 of pf_key_v2.c angelos@ move the argument given to
> pf_key_v2_connection_check(), the one used after free, from
> the stack to the heap:
> 
>Dynamically allocate conn, as this is given to the exchange; cleanup   
>  
>conf space on failure to establish dynamic SA. ok niklas@
> 
> I don't understand the whole magic of function pointers in exchange.c
> but what's interesting is that in his diff he stopped dereferencing
> 'exchange->name'.
> 
> But in pf_key_v2_connection_check() the 'conn' argument is passed as
> 'name' and as 'arg'...  So the diff below fixes Michał's problem.  I'd
> appreciate if more people could test it and check if isakmpd(8) do not
> leaking more memory than it already does.
> 
> Note that this diff do not fix the 'conn' leak introduced in the above
> mentioned commit when a connection exist and exchange_establish() is
> not called.

It turns out that the problem comes from connection_checker().  This
function did not get the fix applied by angelos@ in r1.58 of pf_key_v2.c.

Since 'conn' is given to exchange_establish() it must be allocated
dynamically.

Diff below also prevents a use-after-free in connection_record_passive()
and plugs memory leaks in pf_key_v2_connection_check().

Comments, oks?

Index: connection.c
===
RCS file: /cvs/src/sbin/isakmpd/connection.c,v
retrieving revision 1.37
diff -u -p -r1.37 connection.c
--- connection.c22 Jan 2014 03:09:31 -  1.37
+++ connection.c3 Jul 2017 09:03:53 -
@@ -146,6 +146,7 @@ connection_checker(void *vconn)
 {
struct timeval  now;
struct connection *conn = vconn;
+   char *name;
 
gettimeofday(, 0);
now.tv_sec += conf_get_num("General", "check-interval",
@@ -153,9 +154,16 @@ connection_checker(void *vconn)
conn->ev = timer_add_event("connection_checker",
connection_checker, conn, );
if (!conn->ev)
-   log_print("connection_checker: could not add timer event");
-   if (!ui_daemon_passive)
-   pf_key_v2_connection_check(conn->name);
+   log_print("%s: could not add timer event", __func__);
+   if (ui_daemon_passive)
+   return;
+
+   name = strdup(conn->name);
+   if (!name) {
+   log_print("%s: strdup (\"%s\") failed", __func__, conn->name);
+   return;
+   }
+   pf_key_v2_connection_check(name);
 }
 
 /* Find the connection named NAME.  */
Index: pf_key_v2.c
===
RCS file: /cvs/src/sbin/isakmpd/pf_key_v2.c,v
retrieving revision 1.198
diff -u -p -r1.198 pf_key_v2.c
--- pf_key_v2.c 28 Feb 2017 16:46:27 -  1.198
+++ pf_key_v2.c 3 Jul 2017 09:14:34 -
@@ -2141,9 +2141,11 @@ pf_key_v2_connection_check(char *conn)
LOG_DBG((LOG_SYSDEP, 70,
"pf_key_v2_connection_check: SA for %s missing", conn));
exchange_establish(conn, pf_key_v2_stayalive, conn, 0);
-   } else
+   } else {
LOG_DBG((LOG_SYSDEP, 70, "pf_key_v2_connection_check: "
"SA for %s exists", conn));
+   free(conn);
+   }
 }
 
 /* Handle a PF_KEY lifetime expiration message PMSG.  */
@@ -3144,8 +3146,8 @@ pf_key_v2_acquire(struct pf_key_v2_msg *
conf_end(af, 1);
 
/* Let's rock 'n roll. */
-   pf_key_v2_connection_check(conn);
connection_record_passive(conn);
+   pf_key_v2_connection_check(conn);
conn = 0;
 
/* Fall-through to cleanup. */



Re: systemd compat for doas

2017-07-03 Thread Damien Miller
On Mon, 3 Jul 2017, Franco Fichtner wrote:

> 
> > On 2. Jul 2017, at 8:59 PM, Ted Unangst  wrote:
> > 
> > If the username starts with a digit, but isn't a number, treat it like root.
> 
> I question the simplicity of this patch due to the fact that it leaves
> no head room for further security-related regressions.  Maybe more
> progressive over-engineering of the code is a better course of action.

yeah, where's the dbus integration?



Re: systemd compat for doas

2017-07-03 Thread Franco Fichtner

> On 2. Jul 2017, at 8:59 PM, Ted Unangst  wrote:
> 
> If the username starts with a digit, but isn't a number, treat it like root.

I question the simplicity of this patch due to the fact that it leaves
no head room for further security-related regressions.  Maybe more
progressive over-engineering of the code is a better course of action.

> 
> Index: doas.c
> ===
> RCS file: /cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 doas.c
> --- doas.c27 May 2017 09:51:07 -  1.72
> +++ doas.c2 Jul 2017 18:57:36 -
> @@ -55,8 +55,13 @@ parseuid(const char *s, uid_t *uid)
>   return 0;
>   }
>   *uid = strtonum(s, 0, UID_MAX, );
> - if (errstr)
> + if (errstr) {
> + if (isdigit(*s)) {
> + *uid = 0;
> + return 0;
> + }
>   return -1;
> + }
>   return 0;
> }
> 
> 



Re: dont use the malloc ks_limit in softdeps

2017-07-03 Thread David Gwynne
hrm. ignore this.

On Mon, Jul 03, 2017 at 04:30:11PM +1000, David Gwynne wrote:
> the only thing using ks_limit is the ffs softdep code.
> 
> this cleans that up.
> 
> ok?
> 
> Index: ffs/ffs_softdep.c
> ===
> RCS file: /cvs/src/sys/ufs/ffs/ffs_softdep.c,v
> retrieving revision 1.135
> diff -u -p -r1.135 ffs_softdep.c
> --- ffs/ffs_softdep.c 7 Nov 2016 00:26:33 -   1.135
> +++ ffs/ffs_softdep.c 3 Jul 2017 06:26:41 -
> @@ -1159,12 +1159,7 @@ softdep_initialize(void)
>  
>   LIST_INIT();
>   LIST_INIT(_workitem_pending);
> -#ifdef KMEMSTATS
> - max_softdeps = min (initialvnodes * 8,
> - kmemstats[M_INODEDEP].ks_limit / (2 * sizeof(struct inodedep)));
> -#else
> - max_softdeps = initialvnodes * 4;
> -#endif
> + max_softdeps = initialvnodes * 8;
>   arc4random_buf(_hashkey, sizeof(softdep_hashkey));
>   pagedep_hashtbl = hashinit(initialvnodes / 5, M_PAGEDEP, M_WAITOK,
>   _hash);



dont use the malloc ks_limit in softdeps

2017-07-03 Thread David Gwynne
the only thing using ks_limit is the ffs softdep code.

this cleans that up.

ok?

Index: ffs/ffs_softdep.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_softdep.c,v
retrieving revision 1.135
diff -u -p -r1.135 ffs_softdep.c
--- ffs/ffs_softdep.c   7 Nov 2016 00:26:33 -   1.135
+++ ffs/ffs_softdep.c   3 Jul 2017 06:26:41 -
@@ -1159,12 +1159,7 @@ softdep_initialize(void)
 
LIST_INIT();
LIST_INIT(_workitem_pending);
-#ifdef KMEMSTATS
-   max_softdeps = min (initialvnodes * 8,
-   kmemstats[M_INODEDEP].ks_limit / (2 * sizeof(struct inodedep)));
-#else
-   max_softdeps = initialvnodes * 4;
-#endif
+   max_softdeps = initialvnodes * 8;
arc4random_buf(_hashkey, sizeof(softdep_hashkey));
pagedep_hashtbl = hashinit(initialvnodes / 5, M_PAGEDEP, M_WAITOK,
_hash);