rework ppp and pptp processing in tcpdump
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
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
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)
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
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
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
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
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
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
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)
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)
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)
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
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
> 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
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)
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)
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
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.