rework ppp and pptp processing in tcpdump

2018-02-04 Thread David Gwynne
i think i was looking at gre and pptp packets, and ended up going
down a rabbit hole in ppp parsing.

the original aim was probably to print the ppp payload in pptp
packets, which tcpdump now does. eg:

23:52:00.197893  call 24 seq 7: gre-ppp-payload (gre encap)
23:52:00.198930  call 1 seq 7 ack 7: gre-ppp-payload (gre encap)

now looks like:

23:52:00.197893 20.0.0.2 > 20.0.0.1: pptp callid 24 seq 7: 17.1.1.122 > 
40.0.0.2: icmp: echo request
23:52:00.198930 20.0.0.1 > 20.0.0.2: pptp callid 1 seq 7 ack 7: 40.0.0.2 > 
17.1.1.122: icmp: echo reply

some other changes appeared along the way though:

- better checks for wrong packet lengths when outer protocols (eg,
  pptp and pppoe) have a length field we can look at.

- better ppp control packet parsing, eg, we only look at bytes the
  control packets say we should look at, not at whatever may be left
  in the buffer.

eg, if an ethernet packet comes off the wire with 0 padding up to
60 bytes, the previous ppp code would try to interpret the padding.
so this:

23:43:29.271560 PPPoE-Session
code Session, version 1, type 1, id 0x0011, length 20
LCP: Configure-Request, Max-Rx-Unit=1492, Auth-Prot PAP, 
Magic-Number=33344507, Vendor-Ext

should look like this:

23:43:29.271560 PPPoE-Session
code Session, version 1, type 1, id 0x0011, length 20
LCP Configure-Request Id=0x01: Max-Rx-Unit=1492 Auth-Prot=PAP 
Magic-Number=33344507

- adds support for DLT_PPP_SERIAL

it looks a lot like DLT_PPP, but "upstream" tcpdump handles them
differently. so theyre separate for now.

- add parsing for EAP control packets in ppp.


btw, would anyone mind if i cut the PPPoE output down? out of this:

23:43:29.205560 PPPoE-Session
code Session, version 1, type 1, id 0x0011, length 12
LCP Configure-Request Id=0x01: Magic-Number=100455513
23:44:11.803560 PPPoE-Session
code Session, version 1, type 1, id 0x0011, length 102
fc00:0:2:100::1:1 > fc00::1: icmp6: echo request
23:44:11.808560 PPPoE-Session
code Session, version 1, type 1, id 0x0011, length 102
fc00::1 > fc00:0:2:100::1:1: icmp6: echo reply

the only interesting in this is the id. it could look like:

23:43:29.205560 PPPoE id 0x0011: LCP Configure-Request Id=0x01: 
Magic-Number=100455513
23:44:11.803560 PPPoE id 0x0011: fc00:0:2:100::1:1 > fc00::1: icmp6: echo 
request
23:44:11.808560 PPPoE id 0x0011: fc00::1 > fc00:0:2:100::1:1: icmp6: echo reply

to show the result, here's some output based on parsing pcap files
i found on the internet (thank goodness for privsep/pledge eh):

dlg@m4k1 tcpdump$ for i in ~/tmp/*.cap; do echo == `basename "$i" .cap`  ==; 
echo = stock; sudo tcpdump -nr "$i"; echo = patched; 
/home/dlg/src/usr.sbin/tcpdump/obj/tcpdump -nr "$i"; done  
== GRE ==
= stock
tcpdump: WARNING: snaplen raised from 116 to 8192
22:06:06.434897 0800: 1.1.1.1 > 2.2.2.2: icmp: echo request (gre encap)
22:06:06.442931 0800: 2.2.2.2 > 1.1.1.1: icmp: echo reply (gre encap)
22:06:06.450900 0800: 1.1.1.1 > 2.2.2.2: icmp: echo request (gre encap)
22:06:06.498938 0800: 2.2.2.2 > 1.1.1.1: icmp: echo reply (gre encap)
22:06:06.506904 0800: 1.1.1.1 > 2.2.2.2: icmp: echo request (gre encap)
22:06:06.514914 0800: 2.2.2.2 > 1.1.1.1: icmp: echo reply (gre encap)
22:06:06.522905 0800: 1.1.1.1 > 2.2.2.2: icmp: echo request (gre encap)
22:06:06.570925 0800: 2.2.2.2 > 1.1.1.1: icmp: echo reply (gre encap)
22:06:06.578905 0800: 1.1.1.1 > 2.2.2.2: icmp: echo request (gre encap)
22:06:06.586923 0800: 2.2.2.2 > 1.1.1.1: icmp: echo reply (gre encap)
= patched
tcpdump: WARNING: snaplen raised from 116 to 8192
22:06:06.434897 10.0.0.1 > 10.0.0.2: gre 1.1.1.1 > 2.2.2.2: icmp: echo request
22:06:06.442931 10.0.0.2 > 10.0.0.1: gre 2.2.2.2 > 1.1.1.1: icmp: echo reply
22:06:06.450900 10.0.0.1 > 10.0.0.2: gre 1.1.1.1 > 2.2.2.2: icmp: echo request
22:06:06.498938 10.0.0.2 > 10.0.0.1: gre 2.2.2.2 > 1.1.1.1: icmp: echo reply
22:06:06.506904 10.0.0.1 > 10.0.0.2: gre 1.1.1.1 > 2.2.2.2: icmp: echo request
22:06:06.514914 10.0.0.2 > 10.0.0.1: gre 2.2.2.2 > 1.1.1.1: icmp: echo reply
22:06:06.522905 10.0.0.1 > 10.0.0.2: gre 1.1.1.1 > 2.2.2.2: icmp: echo request
22:06:06.570925 10.0.0.2 > 10.0.0.1: gre 2.2.2.2 > 1.1.1.1: icmp: echo reply
22:06:06.578905 10.0.0.1 > 10.0.0.2: gre 1.1.1.1 > 2.2.2.2: icmp: echo request
22:06:06.586923 10.0.0.2 > 10.0.0.1: gre 2.2.2.2 > 1.1.1.1: icmp: echo reply
== NHRP_registration ==
= stock
tcpdump: WARNING: snaplen raised from 116 to 8192
16:34:45.635442 2001: gre-proto-0x2001 (gre encap)
16:34:45.647483 2001: gre-proto-0x2001 (gre encap)
16:34:45.659433 2001: gre-proto-0x2001 (gre encap)
16:34:45.671422 2001: gre-proto-0x2001 (gre encap)
= patched
tcpdump: WARNING: snaplen raised from 116 to 8192
16:34:45.635442 172.16.25.2 > 172.16.15.2: gre unknown-proto-2001 [tos 0xc0]
16:34:45.647483 172.16.25.2 > 172.16.15.2: gre unknown-proto-2001 [tos 0xc0]
16:34:45.659433 172.16.15.2 > 172.16.25.2: gre unknown-proto-2001 [tos 0xc0]
16:34:45.671422 

Re: athn(4) USB open firmware support

2018-02-04 Thread Kevin Lo
On Sun, Feb 04, 2018 at 09:54:35PM +0100, Stefan Sperling wrote:
> 
> On Sun, Feb 04, 2018 at 11:15:01AM +0100, Stefan Sperling wrote:
> > On Sun, Feb 04, 2018 at 12:01:58AM +0100, Stefan Sperling wrote:
> > > I have also briefly tested hostap with both AR9271 and AR7010 and it
> > > now seems to work well enough to justify removing the BUGS section from
> > > the athn(4) page.
> > 
> > I have to retract the above statement.
> > I've done some more testing and hostap mode still has serious problems.
> > But it can be fixed later. I've already got some ideas about what's wrong.
> 
> Here is a new version with fixed hostap mode.
> 
> There were two major problems for hostap with the previous code:
> 
> 1) The previous code tried to add each new node to the firmware's table
> when an ASSOC request is about to be confirmed. But that is too late.
> We need to add the new client to the firmware's node table already
> when net80211 tries to acknowledge the AUTH request.
> Otherwise our AUTH response will be dropped by the device because it
> has no destination node in the firmware. The client will now keep
> trying to AUTH, and until it has done so it won't try to ASSOC.
> The fix requires a new function hook in net80211 which drivers can set if
> they need to do something before an incoming AUTH request is acknowledged.
> Some client capabilities are not available this early, so this driver
> still needs to update the node's rate set when an ASSOC request is
> acknowledged (otherwise we'd break 11n support).
> 
> 2) There is only room for 8 nodes in the firmware's node table.
> The previous code used a counter to keep track of nodes added/removed,
> but for hostap a counter is insufficient as nodes will show up and leave
> in no particular order.
> What is needed instead is a bitmask which keeps track of free node slots.
> Since there are only 8 slots, we need to be a bit aggressive about removing
> inactive clients when the table is full and a new client wants to connect.
> However, it is still hard for new clients to steal slots which are being
> legitimately used. We rely on net80211's inactivity timer to detect inactive
> clients. It takes about a minute or so for a new client to associate to
> an AP with a full table and no active clients. I think this is acceptable.
> 
> This diff keeps growing Can I commit?

A quick test went fine for me.

Tested in hostap mode and client mode with:
athn0 at uhub0 port 2 configuration 1 interface 0 "ATHEROS USB2.0 WLAN" rev 
2.00/1.08 addr 2
athn0: AR9271 rev 1 (1T1R), ROM rev 13, address xx:xx:xx:xx:xx:xx

In client mode, I see TCP download rates go up to 750KB/s; when running an AP
with this device, TCP download rates for iwn are a little slower.

ok kevlo@



Re: Update ifconfig(8): wpaakms implies wpa option

2018-02-04 Thread Theo de Raadt
I don't see the point of these details.  Documenting every behaviour
makes the manual page hard to read.

> Update the ifconfig(8) man page to state that the wpa option is
> implied when the wpaakms option is set. 
> 
> Changed in ifconfig.c revision 1.354.
> 
> Index: ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.292
> diff -u -p -u -p -r1.292 ifconfig.8
> --- ifconfig.816 Jan 2018 10:33:55 -  1.292
> +++ ifconfig.85 Feb 2018 01:07:30 -
> @@ -963,13 +963,20 @@ In Host AP mode, this will dump the list
>  Enable Wi-Fi Protected Access.
>  WPA is a Wi-Fi Alliance protocol based on the IEEE 802.11i standard.
>  It was designed to enhance the security of wireless networks.
> +Setting the
> +.Cm wpaakms
> +option implies enable WPA.
>  Notice that not all drivers support WPA.
>  Check the driver's manual page to know if this option is supported.
>  .It Cm -wpa
>  Disable Wi-Fi Protected Access.
>  .It Cm wpaakms Ar akm,akm,...
>  Set the comma-separated list of allowed authentication and key management
> -protocols.
> +protocols. The
> +.Cm wpa
> +option is implied and is not required when the 
> +.Cm wpaakms
> +option is set.
>  .Pp
>  The supported values are
>  .Dq psk
> 



Re: ssh: don't close fds multiple times and don't close(-1)

2018-02-04 Thread Damien Miller
ok djm

On Mon, 5 Feb 2018, Theo Buehler wrote:

> In channel_close_fd(), the file descriptors for the socket, stdin,
> stdout and stderr aren't necessarily distinct, so closing them results
> in EBADF. In addition, the diff adds a couple of positivity checks to
> avoid calling close(-1).
> 
> Index: usr.bin/ssh/channels.c
> ===
> RCS file: /var/cvs/src/usr.bin/ssh/channels.c,v
> retrieving revision 1.378
> diff -u -p -r1.378 channels.c
> --- usr.bin/ssh/channels.c23 Jan 2018 05:27:21 -  1.378
> +++ usr.bin/ssh/channels.c24 Jan 2018 00:41:18 -
> @@ -426,10 +426,15 @@ channel_close_fd(struct ssh *ssh, int *f
>  static void
>  channel_close_fds(struct ssh *ssh, Channel *c)
>  {
> + int sock = c->sock, rfd = c->rfd, wfd = c->wfd, efd = c->efd;
> +
>   channel_close_fd(ssh, >sock);
> - channel_close_fd(ssh, >rfd);
> - channel_close_fd(ssh, >wfd);
> - channel_close_fd(ssh, >efd);
> + if (rfd != sock)
> + channel_close_fd(ssh, >rfd);
> + if (wfd != sock && wfd != rfd)
> + channel_close_fd(ssh, >wfd);
> + if (efd != sock && efd != rfd && efd != wfd)
> + channel_close_fd(ssh, >efd);
>  }
>  
>  static void
> Index: usr.bin/ssh/monitor.c
> ===
> RCS file: /var/cvs/src/usr.bin/ssh/monitor.c,v
> retrieving revision 1.178
> diff -u -p -r1.178 monitor.c
> --- usr.bin/ssh/monitor.c 23 Jan 2018 05:27:21 -  1.178
> +++ usr.bin/ssh/monitor.c 24 Jan 2018 00:41:18 -
> @@ -230,8 +230,10 @@ monitor_child_preauth(Authctxt *_authctx
>  
>   debug3("preauth child monitor started");
>  
> - close(pmonitor->m_recvfd);
> - close(pmonitor->m_log_sendfd);
> + if (pmonitor->m_recvfd >= 0)
> + close(pmonitor->m_recvfd);
> + if (pmonitor->m_log_sendfd >= 0)
> + close(pmonitor->m_log_sendfd);
>   pmonitor->m_log_sendfd = pmonitor->m_recvfd = -1;
>  
>   authctxt = _authctxt;
> @@ -298,8 +300,10 @@ monitor_child_preauth(Authctxt *_authctx
>   while (pmonitor->m_log_recvfd != -1 && monitor_read_log(pmonitor) == 0)
>   ;
>  
> - close(pmonitor->m_sendfd);
> - close(pmonitor->m_log_recvfd);
> + if (pmonitor->m_recvfd >= 0)
> + close(pmonitor->m_recvfd);
> + if (pmonitor->m_log_sendfd >= 0)
> + close(pmonitor->m_log_sendfd);
>   pmonitor->m_sendfd = pmonitor->m_log_recvfd = -1;
>  }
>  
> Index: usr.bin/ssh/ssh-pkcs11-client.c
> ===
> RCS file: /var/cvs/src/usr.bin/ssh/ssh-pkcs11-client.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 ssh-pkcs11-client.c
> --- usr.bin/ssh/ssh-pkcs11-client.c   30 May 2017 08:52:19 -  1.7
> +++ usr.bin/ssh/ssh-pkcs11-client.c   23 Jan 2018 00:09:22 -
> @@ -93,7 +93,8 @@ pkcs11_init(int interactive)
>  void
>  pkcs11_terminate(void)
>  {
> - close(fd);
> + if (fd >= 0)
> + close(fd);
>  }
>  
>  static int
> 
> 



Update ifconfig(8): wpaakms implies wpa option

2018-02-04 Thread James Jerkins

Update the ifconfig(8) man page to state that the wpa option is
implied when the wpaakms option is set. 

Changed in ifconfig.c revision 1.354.

Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.292
diff -u -p -u -p -r1.292 ifconfig.8
--- ifconfig.8  16 Jan 2018 10:33:55 -  1.292
+++ ifconfig.8  5 Feb 2018 01:07:30 -
@@ -963,13 +963,20 @@ In Host AP mode, this will dump the list
 Enable Wi-Fi Protected Access.
 WPA is a Wi-Fi Alliance protocol based on the IEEE 802.11i standard.
 It was designed to enhance the security of wireless networks.
+Setting the
+.Cm wpaakms
+option implies enable WPA.
 Notice that not all drivers support WPA.
 Check the driver's manual page to know if this option is supported.
 .It Cm -wpa
 Disable Wi-Fi Protected Access.
 .It Cm wpaakms Ar akm,akm,...
 Set the comma-separated list of allowed authentication and key management
-protocols.
+protocols. The
+.Cm wpa
+option is implied and is not required when the 
+.Cm wpaakms
+option is set.
 .Pp
 The supported values are
 .Dq psk



Re: cron: don't close crontab_fd twice

2018-02-04 Thread Theo Buehler
On Sun, Feb 04, 2018 at 07:19:57PM -0700, Todd C. Miller wrote:
> On Mon, 05 Feb 2018 09:21:32 +1300, Theo Buehler wrote:
> 
> > The load_user() function gets a file descriptor from process_crontab().
> > It fdopen()s it directly and fclose()s the resulting stream. Then
> > process_crontab() closes the stream a second time before exiting.
> >
> > Since crontab_fd is not load_user()'s descriptor, let's dup it before
> > opening the stream.
> 
> Alternately, we could just pass in a FILE * to load_user().

That's much nicer. ok



Re: cron: don't close crontab_fd twice

2018-02-04 Thread Todd C. Miller
On Mon, 05 Feb 2018 09:21:32 +1300, Theo Buehler wrote:

> The load_user() function gets a file descriptor from process_crontab().
> It fdopen()s it directly and fclose()s the resulting stream. Then
> process_crontab() closes the stream a second time before exiting.
>
> Since crontab_fd is not load_user()'s descriptor, let's dup it before
> opening the stream.

Alternately, we could just pass in a FILE * to load_user().

 - todd

Index: usr.sbin/cron/database.c
===
RCS file: /cvs/src/usr.sbin/cron/database.c,v
retrieving revision 1.36
diff -u -p -u -r1.36 database.c
--- usr.sbin/cron/database.c25 Oct 2017 17:08:58 -  1.36
+++ usr.sbin/cron/database.c5 Feb 2018 02:18:37 -
@@ -170,9 +170,10 @@ process_crontab(int dfd, const char *una
struct stat *statbuf, cron_db *new_db, cron_db *old_db)
 {
struct passwd *pw = NULL;
-   int crontab_fd = -1;
+   FILE *crontab_fp = NULL;
user *u, *new_u;
mode_t tabmask, tabperm;
+   int fd;
 
/* Note: pw must remain NULL for system crontab (see below). */
if (fname[0] != '/' && (pw = getpwnam(uname)) == NULL) {
@@ -182,16 +183,20 @@ process_crontab(int dfd, const char *una
goto next_crontab;
}
 
-   crontab_fd = openat(dfd, fname,
-   O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
-   if (crontab_fd < 0) {
+   fd = openat(dfd, fname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
+   if (fd < 0) {
/* crontab not accessible?
 */
syslog(LOG_ERR, "(%s) CAN'T OPEN (%s)", uname, fname);
goto next_crontab;
}
+   if (!(crontab_fp = fdopen(fd, "r"))) {
+   syslog(LOG_ERR, "(%s) FDOPEN (%m)", fname);
+   close(fd);
+   goto next_crontab;
+   }
 
-   if (fstat(crontab_fd, statbuf) < 0) {
+   if (fstat(fileno(crontab_fp), statbuf) < 0) {
syslog(LOG_ERR, "(%s) FSTAT FAILED (%s)", uname, fname);
goto next_crontab;
}
@@ -233,7 +238,7 @@ process_crontab(int dfd, const char *una
syslog(LOG_INFO, "(%s) RELOAD (%s)", uname, fname);
}
 
-   new_u = load_user(crontab_fd, pw, fname);
+   new_u = load_user(crontab_fp, pw, fname);
if (new_u != NULL) {
/* Insert user into the new database and remove from old. */
new_u->mtime = statbuf->st_mtim;
@@ -249,7 +254,7 @@ process_crontab(int dfd, const char *una
}
 
  next_crontab:
-   if (crontab_fd >= 0) {
-   close(crontab_fd);
+   if (crontab_fp != NULL) {
+   fclose(crontab_fp);
}
 }
Index: usr.sbin/cron/funcs.h
===
RCS file: /cvs/src/usr.sbin/cron/funcs.h,v
retrieving revision 1.28
diff -u -p -u -r1.28 funcs.h
--- usr.sbin/cron/funcs.h   14 Nov 2015 13:09:14 -  1.28
+++ usr.sbin/cron/funcs.h   5 Feb 2018 02:18:56 -
@@ -48,7 +48,7 @@ char  *env_get(char *, char **),
**env_copy(char **),
**env_set(char **, char *);
 
-user   *load_user(int, struct passwd *, const char *),
+user   *load_user(FILE *, struct passwd *, const char *),
*find_user(cron_db *, const char *);
 
 entry  *load_entry(FILE *,
Index: usr.sbin/cron/user.c
===
RCS file: /cvs/src/usr.sbin/cron/user.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 user.c
--- usr.sbin/cron/user.c7 Jun 2017 23:36:43 -   1.20
+++ usr.sbin/cron/user.c5 Feb 2018 02:17:51 -
@@ -58,19 +58,14 @@ parse_error(const char *msg)
 }
 
 user *
-load_user(int crontab_fd, struct passwd*pw, const char *name)
+load_user(FILE *file, struct passwd *pw, const char *name)
 {
char envstr[MAX_ENVSTR];
-   FILE *file;
user *u;
entry *e;
int status, save_errno;
char **envp = NULL, **tenvp;
 
-   if (!(file = fdopen(crontab_fd, "r"))) {
-   syslog(LOG_ERR, "(%s) FDOPEN (%m)", name);
-   return (NULL);
-   }
CrontabFilename = name;
LineNumber = 0;
 
@@ -134,6 +129,5 @@ load_user(int crontab_fd, struct passwd 
  done:
if (envp != NULL)
env_free(envp);
-   fclose(file);
return (u);
 }



Re: amd64: much earlier Intel microcode loading

2018-02-04 Thread Mike Larkin
On Mon, Feb 05, 2018 at 11:02:27AM +1300, Patrick Wildt wrote:
> On Wed, Jan 31, 2018 at 05:12:03PM +0100, Patrick Wildt wrote:
> > Hi,
> > 
> > this diff allows us to load the Intel microcode much earlier.  So far
> > we load it after the CPUs have identified and then have to update the
> > CPU flags afterwards.  This is not good since we have to assume that
> > those updates can remove and add instructions and other features.  We
> > need to load it earlier.  The only other option is to have the boot-
> > blocks load the ucode for us.
> > 
> > One issue though is actually loading and passing the binaries that can
> > range from 2k to 90k of binary size.  As far as I know we don't use the
> > lower 16M of memory on amd64, which the bootblocks use as heap.  Thus
> > allocating a buffer for using alloc() should be "fine" if we make sure
> > that we read the ucode after we early enough.  kettenis@ tells me that
> > cpu_startup() is the earliest MD code which can use malloc(9), so that
> > is where we can copy the ucode from the bootloader to kernel land.
> > 
> > I have tested this with efiboot(8), more tests would be appreciated.
> > 
> > Thanks,
> > Patrick
> 
> Updated diff.  RAMDISK compiles and runs through make release without
> any further issues.
> 
> This diff also enables the debug printfs, so people testing this diff
> should see something happening in dmesg.  I will not commit that diff
> with the prints enabled.
> 
> Please report back.  Some more stuff should be seen when doing
> 
>   $ dmesg | grep ucode
> 
> Please not that you have to build and install the bootloader in addtion
> to installing a new kernel.
> 
> Thanks,
> Patrick
> 

I did a single test with sr crypto and didn't see any issues, fwiw.

-ml

> diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c
> index 8a779aa65eb..5ec7e7f6409 100644
> --- a/sys/arch/amd64/amd64/cpu.c
> +++ b/sys/arch/amd64/amd64/cpu.c
> @@ -406,21 +406,24 @@ cpu_attach(struct device *parent, struct device *self, 
> void *aux)
>   printf("(uniprocessor)\n");
>   ci->ci_flags |= CPUF_PRESENT | CPUF_SP | CPUF_PRIMARY;
>   cpu_intr_init(ci);
> +#ifndef SMALL_KERNEL
> + cpu_ucode_apply(ci);
> +#endif
>   identifycpu(ci);
>  #ifdef MTRR
>   mem_range_attach();
>  #endif /* MTRR */
>   cpu_init(ci);
>   cpu_init_mwait(sc);
> -#ifndef SMALL_KERNEL
> - config_mountroot(NULL, cpu_ucode_attachhook);
> -#endif
>   break;
>  
>   case CPU_ROLE_BP:
>   printf("apid %d (boot processor)\n", caa->cpu_apicid);
>   ci->ci_flags |= CPUF_PRESENT | CPUF_BSP | CPUF_PRIMARY;
>   cpu_intr_init(ci);
> +#ifndef SMALL_KERNEL
> + cpu_ucode_apply(ci);
> +#endif
>   identifycpu(ci);
>  #ifdef MTRR
>   mem_range_attach();
> @@ -438,9 +441,6 @@ cpu_attach(struct device *parent, struct device *self, 
> void *aux)
>   ioapic_bsp_id = caa->cpu_apicid;
>  #endif
>   cpu_init_mwait(sc);
> -#ifndef SMALL_KERNEL
> - config_mountroot(NULL, cpu_ucode_attachhook);
> -#endif
>   break;
>  
>   case CPU_ROLE_AP:
> @@ -698,6 +698,7 @@ cpu_hatch(void *v)
>  
>   lapic_enable();
>   lapic_startclock();
> + cpu_ucode_apply(ci);
>  
>   if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
>   /*
> @@ -738,8 +739,6 @@ cpu_hatch(void *v)
>   lldt(0);
>  
>   cpu_init(ci);
> - cpu_ucode_apply(ci);
> - cpu_flags_update(ci);
>  #if NPVBUS > 0
>   pvbus_init_cpu();
>  #endif
> @@ -939,20 +938,3 @@ cpu_activate(struct device *self, int act)
>  
>   return (0);
>  }
> -
> -void
> -cpu_flags_update(struct cpu_info *ci)
> -{
> - uint32_t dummy;
> -
> - /* XXX this update is much later than we want it to be */
> - if (cpuid_level >= 0x07) {
> - CPUID_LEAF(0x7, 0, dummy, dummy, dummy,
> - ci->ci_feature_sefflags_edx);
> - }
> - if (!strcmp(cpu_vendor, "AuthenticAMD") &&
> - ci->ci_pnfeatset >= 0x8008) {
> - CPUID(0x8008, dummy, ci->ci_feature_amdspec_ebx,
> - dummy, dummy);
> - }
> -}
> diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
> index 33d1d38395b..940716c174c 100644
> --- a/sys/arch/amd64/amd64/machdep.c
> +++ b/sys/arch/amd64/amd64/machdep.c
> @@ -241,6 +241,7 @@ bios_diskinfo_t   *bios_diskinfo;
>  bios_memmap_t*bios_memmap;
>  u_int32_tbios_cksumlen;
>  bios_efiinfo_t   *bios_efiinfo;
> +bios_ucode_t *bios_ucode;
>  
>  /*
>   * Size of memory segments, before any memory is stolen.
> @@ -308,6 +309,10 @@ cpu_startup(void)
>  
>   /* Safe for i/o port / memory space allocation to use malloc now. */
>   x86_bus_space_mallocok();
> +
> +#ifndef SMALL_KERNEL
> + cpu_ucode_setup();
> +#endif
>  }
>  
>  /*
> @@ -1886,6 +1891,10 @@ getbootinfo(char 

daily(8): don't fail silently if backup disk is unavailable

2018-02-04 Thread Theo Buehler
After a power failure, my apu2 booted, but its sdmmc controller didn't
attach properly. A few days later I was wondering why I didn't get the
usual dump output from the backup of the root filesystem in my daily
mails.

It turns out that daily(8) fails silently if it can't find the backup
volume. Since this happens due to a failure of some kind or because of
misconfiguration, I suggest that we print an error message, so this can
be easily spotted in the mail.

Index: etc/daily
===
RCS file: /var/cvs/src/etc/daily,v
retrieving revision 1.90
diff -u -p -r1.90 daily
--- etc/daily   10 Jul 2017 11:18:48 -  1.90
+++ etc/daily   4 Feb 2018 22:28:12 -
@@ -90,7 +90,10 @@ while [ "X$ROOTBACKUP" = X1 ]; do
fi
rootbak=${rootbak#/dev/}
bakdisk=${rootbak%%?(.)[a-p]}
-   sysctl -n hw.disknames | grep -Fqw $bakdisk || break
+   if ! sysctl -n hw.disknames | grep -Fqw $bakdisk; then
+   echo "Backup disk '$bakdisk' not available in hw.disknames."
+   break
+   fi
bakpart=${rootbak##$bakdisk?(.)}
OLDIFS=$IFS
IFS=,



Re: amd64: much earlier Intel microcode loading

2018-02-04 Thread Patrick Wildt
On Wed, Jan 31, 2018 at 05:12:03PM +0100, Patrick Wildt wrote:
> Hi,
> 
> this diff allows us to load the Intel microcode much earlier.  So far
> we load it after the CPUs have identified and then have to update the
> CPU flags afterwards.  This is not good since we have to assume that
> those updates can remove and add instructions and other features.  We
> need to load it earlier.  The only other option is to have the boot-
> blocks load the ucode for us.
> 
> One issue though is actually loading and passing the binaries that can
> range from 2k to 90k of binary size.  As far as I know we don't use the
> lower 16M of memory on amd64, which the bootblocks use as heap.  Thus
> allocating a buffer for using alloc() should be "fine" if we make sure
> that we read the ucode after we early enough.  kettenis@ tells me that
> cpu_startup() is the earliest MD code which can use malloc(9), so that
> is where we can copy the ucode from the bootloader to kernel land.
> 
> I have tested this with efiboot(8), more tests would be appreciated.
> 
> Thanks,
> Patrick

Updated diff.  RAMDISK compiles and runs through make release without
any further issues.

This diff also enables the debug printfs, so people testing this diff
should see something happening in dmesg.  I will not commit that diff
with the prints enabled.

Please report back.  Some more stuff should be seen when doing

$ dmesg | grep ucode

Please not that you have to build and install the bootloader in addtion
to installing a new kernel.

Thanks,
Patrick

diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c
index 8a779aa65eb..5ec7e7f6409 100644
--- a/sys/arch/amd64/amd64/cpu.c
+++ b/sys/arch/amd64/amd64/cpu.c
@@ -406,21 +406,24 @@ cpu_attach(struct device *parent, struct device *self, 
void *aux)
printf("(uniprocessor)\n");
ci->ci_flags |= CPUF_PRESENT | CPUF_SP | CPUF_PRIMARY;
cpu_intr_init(ci);
+#ifndef SMALL_KERNEL
+   cpu_ucode_apply(ci);
+#endif
identifycpu(ci);
 #ifdef MTRR
mem_range_attach();
 #endif /* MTRR */
cpu_init(ci);
cpu_init_mwait(sc);
-#ifndef SMALL_KERNEL
-   config_mountroot(NULL, cpu_ucode_attachhook);
-#endif
break;
 
case CPU_ROLE_BP:
printf("apid %d (boot processor)\n", caa->cpu_apicid);
ci->ci_flags |= CPUF_PRESENT | CPUF_BSP | CPUF_PRIMARY;
cpu_intr_init(ci);
+#ifndef SMALL_KERNEL
+   cpu_ucode_apply(ci);
+#endif
identifycpu(ci);
 #ifdef MTRR
mem_range_attach();
@@ -438,9 +441,6 @@ cpu_attach(struct device *parent, struct device *self, void 
*aux)
ioapic_bsp_id = caa->cpu_apicid;
 #endif
cpu_init_mwait(sc);
-#ifndef SMALL_KERNEL
-   config_mountroot(NULL, cpu_ucode_attachhook);
-#endif
break;
 
case CPU_ROLE_AP:
@@ -698,6 +698,7 @@ cpu_hatch(void *v)
 
lapic_enable();
lapic_startclock();
+   cpu_ucode_apply(ci);
 
if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
/*
@@ -738,8 +739,6 @@ cpu_hatch(void *v)
lldt(0);
 
cpu_init(ci);
-   cpu_ucode_apply(ci);
-   cpu_flags_update(ci);
 #if NPVBUS > 0
pvbus_init_cpu();
 #endif
@@ -939,20 +938,3 @@ cpu_activate(struct device *self, int act)
 
return (0);
 }
-
-void
-cpu_flags_update(struct cpu_info *ci)
-{
-   uint32_t dummy;
-
-   /* XXX this update is much later than we want it to be */
-   if (cpuid_level >= 0x07) {
-   CPUID_LEAF(0x7, 0, dummy, dummy, dummy,
-   ci->ci_feature_sefflags_edx);
-   }
-   if (!strcmp(cpu_vendor, "AuthenticAMD") &&
-   ci->ci_pnfeatset >= 0x8008) {
-   CPUID(0x8008, dummy, ci->ci_feature_amdspec_ebx,
-   dummy, dummy);
-   }
-}
diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
index 33d1d38395b..940716c174c 100644
--- a/sys/arch/amd64/amd64/machdep.c
+++ b/sys/arch/amd64/amd64/machdep.c
@@ -241,6 +241,7 @@ bios_diskinfo_t *bios_diskinfo;
 bios_memmap_t  *bios_memmap;
 u_int32_t  bios_cksumlen;
 bios_efiinfo_t *bios_efiinfo;
+bios_ucode_t   *bios_ucode;
 
 /*
  * Size of memory segments, before any memory is stolen.
@@ -308,6 +309,10 @@ cpu_startup(void)
 
/* Safe for i/o port / memory space allocation to use malloc now. */
x86_bus_space_mallocok();
+
+#ifndef SMALL_KERNEL
+   cpu_ucode_setup();
+#endif
 }
 
 /*
@@ -1886,6 +1891,10 @@ getbootinfo(char *bootinfo, int bootinfo_size)
docninit++;
break;
 
+   case BOOTARG_UCODE:
+   bios_ucode = (bios_ucode_t *)q->ba_arg;
+   break;
+
default:
 #ifdef BOOTINFO_DEBUG
printf(" unsupported arg (%d) %p", 

ssh: don't close fds multiple times and don't close(-1)

2018-02-04 Thread Theo Buehler
In channel_close_fd(), the file descriptors for the socket, stdin,
stdout and stderr aren't necessarily distinct, so closing them results
in EBADF. In addition, the diff adds a couple of positivity checks to
avoid calling close(-1).

Index: usr.bin/ssh/channels.c
===
RCS file: /var/cvs/src/usr.bin/ssh/channels.c,v
retrieving revision 1.378
diff -u -p -r1.378 channels.c
--- usr.bin/ssh/channels.c  23 Jan 2018 05:27:21 -  1.378
+++ usr.bin/ssh/channels.c  24 Jan 2018 00:41:18 -
@@ -426,10 +426,15 @@ channel_close_fd(struct ssh *ssh, int *f
 static void
 channel_close_fds(struct ssh *ssh, Channel *c)
 {
+   int sock = c->sock, rfd = c->rfd, wfd = c->wfd, efd = c->efd;
+
channel_close_fd(ssh, >sock);
-   channel_close_fd(ssh, >rfd);
-   channel_close_fd(ssh, >wfd);
-   channel_close_fd(ssh, >efd);
+   if (rfd != sock)
+   channel_close_fd(ssh, >rfd);
+   if (wfd != sock && wfd != rfd)
+   channel_close_fd(ssh, >wfd);
+   if (efd != sock && efd != rfd && efd != wfd)
+   channel_close_fd(ssh, >efd);
 }
 
 static void
Index: usr.bin/ssh/monitor.c
===
RCS file: /var/cvs/src/usr.bin/ssh/monitor.c,v
retrieving revision 1.178
diff -u -p -r1.178 monitor.c
--- usr.bin/ssh/monitor.c   23 Jan 2018 05:27:21 -  1.178
+++ usr.bin/ssh/monitor.c   24 Jan 2018 00:41:18 -
@@ -230,8 +230,10 @@ monitor_child_preauth(Authctxt *_authctx
 
debug3("preauth child monitor started");
 
-   close(pmonitor->m_recvfd);
-   close(pmonitor->m_log_sendfd);
+   if (pmonitor->m_recvfd >= 0)
+   close(pmonitor->m_recvfd);
+   if (pmonitor->m_log_sendfd >= 0)
+   close(pmonitor->m_log_sendfd);
pmonitor->m_log_sendfd = pmonitor->m_recvfd = -1;
 
authctxt = _authctxt;
@@ -298,8 +300,10 @@ monitor_child_preauth(Authctxt *_authctx
while (pmonitor->m_log_recvfd != -1 && monitor_read_log(pmonitor) == 0)
;
 
-   close(pmonitor->m_sendfd);
-   close(pmonitor->m_log_recvfd);
+   if (pmonitor->m_recvfd >= 0)
+   close(pmonitor->m_recvfd);
+   if (pmonitor->m_log_sendfd >= 0)
+   close(pmonitor->m_log_sendfd);
pmonitor->m_sendfd = pmonitor->m_log_recvfd = -1;
 }
 
Index: usr.bin/ssh/ssh-pkcs11-client.c
===
RCS file: /var/cvs/src/usr.bin/ssh/ssh-pkcs11-client.c,v
retrieving revision 1.7
diff -u -p -r1.7 ssh-pkcs11-client.c
--- usr.bin/ssh/ssh-pkcs11-client.c 30 May 2017 08:52:19 -  1.7
+++ usr.bin/ssh/ssh-pkcs11-client.c 23 Jan 2018 00:09:22 -
@@ -93,7 +93,8 @@ pkcs11_init(int interactive)
 void
 pkcs11_terminate(void)
 {
-   close(fd);
+   if (fd >= 0)
+   close(fd);
 }
 
 static int



Re: ospfd: depend on interface (new feature)

2018-02-04 Thread Stuart Henderson
On 2018/02/04 23:04, Kapetanakis Giannis wrote:
> On 04/02/18 17:52, Stuart Henderson wrote:
> > On 2018/02/04 02:56, Kapetanakis Giannis wrote:
> > > On 04/02/18 01:42, Remi Locherer wrote:
> > > > Hi
> > > > 
> > > > This adds a new feature to ospfd: depend on interface.
> > > > 
> > > If I understand this right, someone could use this combined with pfsync,
> > > to wait for states full sync before switching routes from backup to 
> > > master?
> > > 
> > > nice :)
> > > 
> > > G
> > > 
> > > 
> > I'm not sure pfsync specifically handles that, but as long as you
> > make sure the interface is not master at boot (e.g. carpdemote in
> > hostname.carpX) and sleep and -carpdemote in rc.local, it will have that
> > effect.
> > 
> > Nice thing here is when you combine it with bgpd's demote handling.
> > That way you can avoid feeding default to OSPF until BGP sessions are up.
> 
> I'm not talking about "depend on pfsync"
> 
> Since carp waits for pfync demotion counter to become master,
> if ospf is waiting for carp (which is waiting for pfsync) then eventually
> you have the same effect don't you?
> 
> You can even use an unused carp interface just for depending on it's status.

Oh, looking at the code it seems you're right. Seems undocumented though.



Re: ospfd: depend on interface (new feature)

2018-02-04 Thread Kapetanakis Giannis

On 04/02/18 17:52, Stuart Henderson wrote:

On 2018/02/04 02:56, Kapetanakis Giannis wrote:

On 04/02/18 01:42, Remi Locherer wrote:

Hi

This adds a new feature to ospfd: depend on interface.


If I understand this right, someone could use this combined with pfsync,
to wait for states full sync before switching routes from backup to master?

nice :)

G



I'm not sure pfsync specifically handles that, but as long as you
make sure the interface is not master at boot (e.g. carpdemote in
hostname.carpX) and sleep and -carpdemote in rc.local, it will have that
effect.

Nice thing here is when you combine it with bgpd's demote handling.
That way you can avoid feeding default to OSPF until BGP sessions are up.


I'm not talking about "depend on pfsync"

Since carp waits for pfync demotion counter to become master,
if ospf is waiting for carp (which is waiting for pfsync) then 
eventually you have the same effect don't you?


You can even use an unused carp interface just for depending on it's status.

G




Re: athn(4) USB open firmware support

2018-02-04 Thread Stefan Sperling
On Sun, Feb 04, 2018 at 11:15:01AM +0100, Stefan Sperling wrote:
> On Sun, Feb 04, 2018 at 12:01:58AM +0100, Stefan Sperling wrote:
> > I have also briefly tested hostap with both AR9271 and AR7010 and it
> > now seems to work well enough to justify removing the BUGS section from
> > the athn(4) page.
> 
> I have to retract the above statement.
> I've done some more testing and hostap mode still has serious problems.
> But it can be fixed later. I've already got some ideas about what's wrong.

Here is a new version with fixed hostap mode.

There were two major problems for hostap with the previous code:

1) The previous code tried to add each new node to the firmware's table
when an ASSOC request is about to be confirmed. But that is too late.
We need to add the new client to the firmware's node table already
when net80211 tries to acknowledge the AUTH request.
Otherwise our AUTH response will be dropped by the device because it
has no destination node in the firmware. The client will now keep
trying to AUTH, and until it has done so it won't try to ASSOC.
The fix requires a new function hook in net80211 which drivers can set if
they need to do something before an incoming AUTH request is acknowledged.
Some client capabilities are not available this early, so this driver
still needs to update the node's rate set when an ASSOC request is
acknowledged (otherwise we'd break 11n support).

2) There is only room for 8 nodes in the firmware's node table.
The previous code used a counter to keep track of nodes added/removed,
but for hostap a counter is insufficient as nodes will show up and leave
in no particular order.
What is needed instead is a bitmask which keeps track of free node slots.
Since there are only 8 slots, we need to be a bit aggressive about removing
inactive clients when the table is full and a new client wants to connect.
However, it is still hard for new clients to steal slots which are being
legitimately used. We rely on net80211's inactivity timer to detect inactive
clients. It takes about a minute or so for a new client to associate to
an AP with a full table and no active clients. I think this is acceptable.

This diff keeps growing Can I commit?

Index: dev/usb/if_athn_usb.c
===
RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
retrieving revision 1.48
diff -u -p -r1.48 if_athn_usb.c
--- dev/usb/if_athn_usb.c   26 Oct 2017 15:00:28 -  1.48
+++ dev/usb/if_athn_usb.c   4 Feb 2018 20:27:26 -
@@ -130,17 +130,27 @@ void  athn_usb_newstate_cb(struct athn_u
 void   athn_usb_newassoc(struct ieee80211com *,
struct ieee80211_node *, int);
 void   athn_usb_newassoc_cb(struct athn_usb_softc *, void *);
-void   athn_usb_node_leave(struct ieee80211com *,
+struct ieee80211_node *athn_usb_node_alloc(struct ieee80211com *);
+void   athn_usb_count_active_sta(void *, struct ieee80211_node *);
+void   athn_usb_newauth_cb(struct athn_usb_softc *, void *);
+intathn_usb_newauth(struct ieee80211com *,
+   struct ieee80211_node *, int, uint16_t);
+void   athn_usb_node_free(struct ieee80211com *,
struct ieee80211_node *);
-void   athn_usb_node_leave_cb(struct athn_usb_softc *, void *);
+void   athn_usb_node_free_cb(struct athn_usb_softc *, void *);
 intathn_usb_ampdu_tx_start(struct ieee80211com *,
struct ieee80211_node *, uint8_t);
 void   athn_usb_ampdu_tx_start_cb(struct athn_usb_softc *, void *);
 void   athn_usb_ampdu_tx_stop(struct ieee80211com *,
struct ieee80211_node *, uint8_t);
 void   athn_usb_ampdu_tx_stop_cb(struct athn_usb_softc *, void *);
+void   athn_usb_clean_nodes(void *, struct ieee80211_node *);
 intathn_usb_create_node(struct athn_usb_softc *,
struct ieee80211_node *);
+intathn_usb_node_set_rates(struct athn_usb_softc *,
+   struct ieee80211_node *);
+intathn_usb_remove_node(struct athn_usb_softc *,
+   struct ieee80211_node *);
 void   athn_usb_rx_enable(struct athn_softc *);
 intathn_set_chan(struct athn_softc *, struct ieee80211_channel *,
struct ieee80211_channel *);
@@ -159,6 +169,7 @@ voidathn_usb_delete_key_cb(struct athn
 void   athn_usb_bcneof(struct usbd_xfer *, void *,
usbd_status);
 void   athn_usb_swba(struct athn_usb_softc *);
+void   athn_usb_tx_status(void *, struct ieee80211_node *);
 void   athn_usb_rx_wmi_ctrl(struct athn_usb_softc *, uint8_t *, int);
 void   athn_usb_intr(struct usbd_xfer *, void *,
usbd_status);
@@ -319,8 +330,13 @@ athn_usb_attachhook(struct device *self)
ifp->if_ioctl = athn_usb_ioctl;
ifp->if_start = 

Re: sleep(1): better error messages

2018-02-04 Thread Theo de Raadt
> Exiting with EINVAL is really unusual.  It went into r1.9.  Unless
> I'm missing something I think we can just exit with 1, which is
> consistent with what other utilities do.

No kidding, that's a definate bug.



cron: don't close crontab_fd twice

2018-02-04 Thread Theo Buehler
The load_user() function gets a file descriptor from process_crontab().
It fdopen()s it directly and fclose()s the resulting stream. Then
process_crontab() closes the stream a second time before exiting.

Since crontab_fd is not load_user()'s descriptor, let's dup it before
opening the stream.

Index: usr.sbin/cron/user.c
===
RCS file: /var/cvs/src/usr.sbin/cron/user.c,v
retrieving revision 1.20
diff -u -p -r1.20 user.c
--- usr.sbin/cron/user.c7 Jun 2017 23:36:43 -   1.20
+++ usr.sbin/cron/user.c18 Jan 2018 23:14:20 -
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include   /* for structs.h */
+#include 
 
 #include "macros.h"
 #include "structs.h"
@@ -64,11 +65,16 @@ load_user(int crontab_fd, struct passwd 
FILE *file;
user *u;
entry *e;
-   int status, save_errno;
+   int fd, status, save_errno;
char **envp = NULL, **tenvp;
 
-   if (!(file = fdopen(crontab_fd, "r"))) {
+   if ((fd = dup(crontab_fd)) == -1) {
+   syslog(LOG_ERR, "(%s) DUP (%m)", name);
+   return (NULL);
+   }
+   if (!(file = fdopen(fd, "r"))) {
syslog(LOG_ERR, "(%s) FDOPEN (%m)", name);
+   close(fd);
return (NULL);
}
CrontabFilename = name;



Re: ospfd: depend on interface (new feature)

2018-02-04 Thread Stuart Henderson
On 2018/02/04 02:56, Kapetanakis Giannis wrote:
> On 04/02/18 01:42, Remi Locherer wrote:
> > Hi
> > 
> > This adds a new feature to ospfd: depend on interface.
> > 
> > A ospfd.conf using it looks like this:
> > 
> > --%<--
> > redistribute default depend on carp0
> > area 0.0.0.0 {
> > interface em2 { depend on carp0 }
> > [...]
> > }
> > --%<--
> > 
> > This router would send out the default route and the em2 network with
> > default metrics as long as carp0 is master. When carp0 becomes backup these
> > routes are advertised with metric 65535.
> > 
> > "depend on" can also be used with other interface types than carp.
> > 
> > This diff was started by benno@ at p2k17 (redistribute and config parser).
> > I added the interface part. jca@ contributed several improvements.
> > 
> > Comments, OKs?
> > 
> > Remi
> 
> If I understand this right, someone could use this combined with pfsync,
> to wait for states full sync before switching routes from backup to master?
> 
> nice :)
> 
> G
> 
> 

I'm not sure pfsync specifically handles that, but as long as you
make sure the interface is not master at boot (e.g. carpdemote in
hostname.carpX) and sleep and -carpdemote in rc.local, it will have that
effect.

Nice thing here is when you combine it with bgpd's demote handling.
That way you can avoid feeding default to OSPF until BGP sessions are up.



Re: ospfd: depend on interface (new feature)

2018-02-04 Thread Remi Locherer
On Sun, Feb 04, 2018 at 05:19:59AM +0100, Claudio Jeker wrote:
> On Sun, Feb 04, 2018 at 12:42:22AM +0100, Remi Locherer wrote:
> > Hi
> > 
> > This adds a new feature to ospfd: depend on interface.
> > 
> > A ospfd.conf using it looks like this:
> > 
> > --%<--
> > redistribute default depend on carp0
> > area 0.0.0.0 {
> > interface em2 { depend on carp0 }
> > [...]
> > }
> > --%<--
> > 
> > This router would send out the default route and the em2 network with
> > default metrics as long as carp0 is master. When carp0 becomes backup these
> > routes are advertised with metric 65535.
> > 
> > "depend on" can also be used with other interface types than carp.
> > 
> > This diff was started by benno@ at p2k17 (redistribute and config parser).
> > I added the interface part. jca@ contributed several improvements.
> > 
> > Comments, OKs?
> > 
> > Remi


> > +   *metric = (depend_ok) ? r->metric : MAX_METRIC;
>   ^ ^
> Please remove the () around depend_ok here. The () are not needed. 
> There is many more of these.

updated diff below with () removed around depend_ok.


Index: ospfd.c
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.94
diff -u -p -r1.94 ospfd.c
--- ospfd.c 24 Jan 2017 04:24:25 -  1.94
+++ ospfd.c 4 Feb 2018 11:25:36 -
@@ -29,6 +29,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -512,18 +513,30 @@ imsg_compose_event(struct imsgev *iev, u
 int
 ospf_redistribute(struct kroute *kr, u_int32_t *metric)
 {
+   struct in_addr   addr;
+   struct kif  *kif;
struct redistribute *r;
-   u_int8_t is_default = 0;
+   int  is_default = 0, depend_ok = 1;
+
+   bzero(, sizeof(addr));
 
/* only allow 0.0.0.0/0 via REDIST_DEFAULT */
if (kr->prefix.s_addr == INADDR_ANY && kr->prefixlen == 0)
is_default = 1;
 
SIMPLEQ_FOREACH(r, _conf->redist_list, entry) {
+   if (r->dependon[0] != '\0') {
+   if ((kif = kif_findname(r->dependon, addr, NULL)))
+   depend_ok = ifstate_is_up(kif);
+   else
+   depend_ok = 0;
+   } else 
+   depend_ok = 1;
+
switch (r->type & ~REDIST_NO) {
case REDIST_LABEL:
if (kr->rtlabel == r->label) {
-   *metric = r->metric;
+   *metric = depend_ok ? r->metric : MAX_METRIC;
return (r->type & REDIST_NO ? 0 : 1);
}
break;
@@ -538,7 +551,7 @@ ospf_redistribute(struct kroute *kr, u_i
if (kr->flags & F_DYNAMIC)
continue;
if (kr->flags & F_STATIC) {
-   *metric = r->metric;
+   *metric = depend_ok ? r->metric : MAX_METRIC;
return (r->type & REDIST_NO ? 0 : 1);
}
break;
@@ -548,7 +561,7 @@ ospf_redistribute(struct kroute *kr, u_i
if (kr->flags & F_DYNAMIC)
continue;
if (kr->flags & F_CONNECTED) {
-   *metric = r->metric;
+   *metric = depend_ok ? r->metric : MAX_METRIC;
return (r->type & REDIST_NO ? 0 : 1);
}
break;
@@ -559,7 +572,8 @@ ospf_redistribute(struct kroute *kr, u_i
if (r->addr.s_addr == INADDR_ANY &&
r->mask.s_addr == INADDR_ANY) {
if (is_default) {
-   *metric = r->metric;
+   *metric = depend_ok ? r->metric :
+   MAX_METRIC;
return (r->type & REDIST_NO ? 0 : 1);
} else
return (0);
@@ -568,13 +582,13 @@ ospf_redistribute(struct kroute *kr, u_i
if ((kr->prefix.s_addr & r->mask.s_addr) ==
(r->addr.s_addr & r->mask.s_addr) &&
kr->prefixlen >= mask2prefixlen(r->mask.s_addr)) {
-   *metric = r->metric;
+   *metric = depend_ok ? r->metric : MAX_METRIC;
return (r->type & REDIST_NO ? 0 : 1);
}
break;
case REDIST_DEFAULT:
if (is_default) {
-

Re: athn(4) USB open firmware support

2018-02-04 Thread Stefan Sperling
On Sun, Feb 04, 2018 at 12:01:58AM +0100, Stefan Sperling wrote:
> I have also briefly tested hostap with both AR9271 and AR7010 and it
> now seems to work well enough to justify removing the BUGS section from
> the athn(4) page.

I have to retract the above statement.
I've done some more testing and hostap mode still has serious problems.
But it can be fixed later. I've already got some ideas about what's wrong.