Re: iwm(4): use new firmware images with fragattack fixes
On Tue, May 25, 2021 at 03:48:16PM +0200, Stefan Sperling wrote: > > This patch allows iwm(4) to use new firmware images which are part > of the iwm-20210512 firmware package, available via fw_update (you > need to run fw_update *before* booting with this patch). > > The new firmware images were published right after the fragattacks > embargo period ended. An advisory was published by Intel: > https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00473.html > > I have tested the following cards successfully: > 7265, 8265, 9260, 9560 > > I cannot test any other devices myself right now; help with testing this > patch would be much appreciated. It is likely that all devices will work. > But I would still prefer to see test reports for the following cards before > committing this patch: > 7260, 3160, 3165, 3168, 8260 > > The 7260 and 3160 cards did not receive firmware updates from Intel. Which > could be good or bad news, depending on whether the devices are vulnerable > to fragattacks. The fragattacks test tools might shed light on this if you > are curious. I'd be interested to learn more if you have test results or > any other solid information regarding this. Tested on: iwm0 at pci3 dev 0 function 0 "Intel Dual Band Wireless-AC 3168" rev 0x10, msi iwm0: hw rev 0x220, fw ver 29.198743027.0, address b0:35:9f:xx:xx:xx $ pkg_info -I iwm-firmware iwm-firmware-20210512 firmware binary images for iwm(4) driver No regressions under normal use. Thanks!
Re: smtpd: unused code
On Tue, 25 May 2021 22:50:32 +0200, Eric Faurot wrote: > This diff removes more unused code. OK millert@ - todd
smtpd: unused code
This diff removes more unused code.
Eric.
Index: config.c
===
RCS file: /cvs/src/usr.sbin/smtpd/config.c,v
retrieving revision 1.55
diff -u -p -r1.55 config.c
--- config.c9 Apr 2021 16:43:43 - 1.55
+++ config.c25 May 2021 20:31:16 -
@@ -285,7 +285,6 @@ purge_config(uint8_t what)
while (dict_poproot(env->sc_pki_dict, (void **))) {
freezero(p->pki_cert, p->pki_cert_len);
freezero(p->pki_key, p->pki_key_len);
- EVP_PKEY_free(p->pki_pkey);
free(p);
}
free(env->sc_pki_dict);
@@ -298,8 +297,6 @@ purge_config(uint8_t what)
p->pki_cert = NULL;
freezero(p->pki_key, p->pki_key_len);
p->pki_key = NULL;
- EVP_PKEY_free(p->pki_pkey);
- p->pki_pkey = NULL;
}
}
}
Index: ssl.c
===
RCS file: /cvs/src/usr.sbin/smtpd/ssl.c,v
retrieving revision 1.94
diff -u -p -r1.94 ssl.c
--- ssl.c 5 Mar 2021 12:37:32 - 1.94
+++ ssl.c 25 May 2021 20:39:30 -
@@ -65,41 +65,7 @@ ssl_init(void)
inited = 1;
}
-int
-ssl_setup(SSL_CTX **ctxp, struct pki *pki,
-int (*sni_cb)(SSL *,int *,void *), const char *ciphers)
-{
- SSL_CTX *ctx;
- uint8_t sid[SSL_MAX_SID_CTX_LENGTH];
-
- ctx = ssl_ctx_create(pki->pki_name, pki->pki_cert, pki->pki_cert_len,
ciphers);
-
- /*
-* Set session ID context to a random value. We don't support
-* persistent caching of sessions so it is OK to set a temporary
-* session ID context that is valid during run time.
-*/
- arc4random_buf(sid, sizeof(sid));
- if (!SSL_CTX_set_session_id_context(ctx, sid, sizeof(sid)))
- goto err;
-
- if (sni_cb)
- SSL_CTX_set_tlsext_servername_callback(ctx, sni_cb);
-
- SSL_CTX_set_dh_auto(ctx, pki->pki_dhe);
-
- SSL_CTX_set_ecdh_auto(ctx, 1);
-
- *ctxp = ctx;
- return 1;
-
-err:
- SSL_CTX_free(ctx);
- ssl_error("ssl_setup");
- return 0;
-}
-
-char *
+static char *
ssl_load_file(const char *name, off_t *len, mode_t perm)
{
struct stat st;
@@ -177,7 +143,7 @@ end:
return ret;
}
-char *
+static char *
ssl_load_key(const char *name, off_t *len, char *pass, mode_t perm, const char
*pkiname)
{
FILE*fp = NULL;
@@ -253,53 +219,6 @@ fail:
return (NULL);
}
-SSL_CTX *
-ssl_ctx_create(const char *pkiname, char *cert, off_t cert_len, const char
*ciphers)
-{
- SSL_CTX *ctx;
- size_t pkinamelen = 0;
-
- ctx = SSL_CTX_new(SSLv23_method());
- if (ctx == NULL) {
- ssl_error("ssl_ctx_create");
- fatal("ssl_ctx_create: could not create SSL context");
- }
-
- SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF);
- SSL_CTX_set_timeout(ctx, SSL_SESSION_TIMEOUT);
- SSL_CTX_set_options(ctx,
- SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TICKET);
- SSL_CTX_set_options(ctx,
- SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION);
- SSL_CTX_set_options(ctx, SSL_OP_NO_CLIENT_RENEGOTIATION);
- SSL_CTX_set_options(ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
-
- if (ciphers == NULL)
- ciphers = SSL_CIPHERS;
- if (!SSL_CTX_set_cipher_list(ctx, ciphers)) {
- ssl_error("ssl_ctx_create");
- fatal("ssl_ctx_create: could not set cipher list");
- }
-
- if (cert != NULL) {
- if (pkiname != NULL)
- pkinamelen = strlen(pkiname) + 1;
- if (!SSL_CTX_use_certificate_chain_mem(ctx, cert, cert_len)) {
- ssl_error("ssl_ctx_create");
- fatal("ssl_ctx_create: invalid certificate chain");
- } else if (!ssl_ctx_fake_private_key(ctx,
- pkiname, pkinamelen, cert, cert_len, NULL, NULL)) {
- ssl_error("ssl_ctx_create");
- fatal("ssl_ctx_create: could not fake private key");
- } else if (!SSL_CTX_check_private_key(ctx)) {
- ssl_error("ssl_ctx_create");
- fatal("ssl_ctx_create: invalid private key");
- }
- }
-
- return (ctx);
-}
-
int
ssl_load_certificate(struct pki *p, const char *pathname)
{
@@ -329,19 +248,6 @@ ssl_load_cafile(struct ca *c, const char
return 1;
}
-const char *
-ssl_to_text(const SSL *ssl)
-{
- static char buf[256];
-
- (void)snprintf(buf, sizeof buf, "%s:%s:%d",
- SSL_get_version(ssl),
- SSL_get_cipher_name(ssl),
- SSL_get_cipher_bits(ssl, NULL));
-
- return (buf);
Re: vga(4): fix vga_doswitch() declaration
> Date: Tue, 25 May 2021 13:53:57 -0500
> From: Scott Cheloha
>
> Timeout callback functions should be void (*)(void *). I'd rather not
> cast in order to shove the function pointer into timeout_set(9).
>
> ok?
sure, ok kettenis@
> Index: vga.c
> ===
> RCS file: /cvs/src/sys/dev/ic/vga.c,v
> retrieving revision 1.73
> diff -u -p -r1.73 vga.c
> --- vga.c 25 May 2020 09:55:48 - 1.73
> +++ vga.c 25 May 2021 18:38:59 -
> @@ -254,7 +254,7 @@ void vga_scrollback(void *, void *, int)
> void vga_burner(void *v, u_int on, u_int flags);
> int vga_getchar(void *, int, int, struct wsdisplay_charcell *);
>
> -void vga_doswitch(struct vga_config *);
> +void vga_doswitch(void *);
>
> const struct wsdisplay_accessops vga_accessops = {
> .ioctl = vga_ioctl,
> @@ -763,8 +763,7 @@ vga_show_screen(void *v, void *cookie, i
> vc->switchcb = cb;
> vc->switchcbarg = cbarg;
> if (cb) {
> - timeout_set(>vc_switch_timeout,
> - (void(*)(void *))vga_doswitch, vc);
> + timeout_set(>vc_switch_timeout, vga_doswitch, vc);
> timeout_add(>vc_switch_timeout, 0);
> return (EAGAIN);
> }
> @@ -774,8 +773,9 @@ vga_show_screen(void *v, void *cookie, i
> }
>
> void
> -vga_doswitch(struct vga_config *vc)
> +vga_doswitch(void *arg)
> {
> + struct vga_config *vc = arg;
> struct vgascreen *scr, *oldscr;
> struct vga_handle *vh = >hdl;
> const struct wsscreen_descr *type;
>
>
vga(4): fix vga_doswitch() declaration
Timeout callback functions should be void (*)(void *). I'd rather not
cast in order to shove the function pointer into timeout_set(9).
ok?
Index: vga.c
===
RCS file: /cvs/src/sys/dev/ic/vga.c,v
retrieving revision 1.73
diff -u -p -r1.73 vga.c
--- vga.c 25 May 2020 09:55:48 - 1.73
+++ vga.c 25 May 2021 18:38:59 -
@@ -254,7 +254,7 @@ voidvga_scrollback(void *, void *, int)
void vga_burner(void *v, u_int on, u_int flags);
intvga_getchar(void *, int, int, struct wsdisplay_charcell *);
-void vga_doswitch(struct vga_config *);
+void vga_doswitch(void *);
const struct wsdisplay_accessops vga_accessops = {
.ioctl = vga_ioctl,
@@ -763,8 +763,7 @@ vga_show_screen(void *v, void *cookie, i
vc->switchcb = cb;
vc->switchcbarg = cbarg;
if (cb) {
- timeout_set(>vc_switch_timeout,
- (void(*)(void *))vga_doswitch, vc);
+ timeout_set(>vc_switch_timeout, vga_doswitch, vc);
timeout_add(>vc_switch_timeout, 0);
return (EAGAIN);
}
@@ -774,8 +773,9 @@ vga_show_screen(void *v, void *cookie, i
}
void
-vga_doswitch(struct vga_config *vc)
+vga_doswitch(void *arg)
{
+ struct vga_config *vc = arg;
struct vgascreen *scr, *oldscr;
struct vga_handle *vh = >hdl;
const struct wsscreen_descr *type;
Re: panic(9): set panicstr atomically
gwes wrote: > On 5/25/21 10:26 AM, Theo de Raadt wrote: > > Alexander Bluhm wrote: > > > >> On Tue, May 25, 2021 at 04:15:26PM +0200, Mark Kettenis wrote: > >>> Wouldn't be too hard. But unless you're on a serial console, that > >>> will probably be more than a screenful of information, so not terribly > >>> useful. > >> The most important things must fit on the first VGA screen. Then > >> user can make a photo. We ship products to customers who only > >> understand this simple step. And when the machine panics, the admin > >> might also panic. > > Matches my experience. > > > > It will be very difficult to show state of multiple cpus on a small > > screen. On the other hand, only showing the first-panic can hide so > > much. These garbled reports are actually highlighting a not-simple > > case. There is big information in the garble. > > > Jumping in, sorry, > > Some sort of pseudo-ordering where each CPU -mostly- gets to put out > more than one character at a time - 2, 3, ... one line max? > No guaranteed separation/ordering but might make decipherment easier. > > No communication or dependence between CPUs needed. > > Each CPU backs off for a -very- short time after N characters. > Nanoseconds or microseconds should work. Initial delay M * CPU ? > Spin after N? If clock is ticking, spin for x ticks? > Formatting/output time per burst could (maybe) give a little fairness. > > Apologies if this is nonsense or has been covered already. That is not the kind of code you want anywhere near the "heading to ddb" situation. We are heading to ddb because something in the 'software machine state' is busted. We need to rely upon FEWER COMPONENTS still operating normally, not more!
Re: panic(9): set panicstr atomically
On 5/25/21 10:26 AM, Theo de Raadt wrote: Alexander Bluhm wrote: On Tue, May 25, 2021 at 04:15:26PM +0200, Mark Kettenis wrote: Wouldn't be too hard. But unless you're on a serial console, that will probably be more than a screenful of information, so not terribly useful. The most important things must fit on the first VGA screen. Then user can make a photo. We ship products to customers who only understand this simple step. And when the machine panics, the admin might also panic. Matches my experience. It will be very difficult to show state of multiple cpus on a small screen. On the other hand, only showing the first-panic can hide so much. These garbled reports are actually highlighting a not-simple case. There is big information in the garble. Jumping in, sorry, Some sort of pseudo-ordering where each CPU -mostly- gets to put out more than one character at a time - 2, 3, ... one line max? No guaranteed separation/ordering but might make decipherment easier. No communication or dependence between CPUs needed. Each CPU backs off for a -very- short time after N characters. Nanoseconds or microseconds should work. Initial delay M * CPU ? Spin after N? If clock is ticking, spin for x ticks? Formatting/output time per burst could (maybe) give a little fairness. Apologies if this is nonsense or has been covered already. Geoff Steckel
Re: iwm(4): use new firmware images with fragattack fixes
So far this is working on my X1 gen2: iwm0 at pci2 dev 0 function 0 "Intel AC 7260" rev 0x83, msi iwm0: hw rev 0x140, fw ver 17.3216344376.0 -- I'm not entirely sure you are real.
Re: iwm(4): use new firmware images with fragattack fixes
On Tue 25/05/2021 15:48, Stefan Sperling wrote: > This patch allows iwm(4) to use new firmware images which are part > of the iwm-20210512 firmware package, available via fw_update (you > need to run fw_update *before* booting with this patch). > > The new firmware images were published right after the fragattacks > embargo period ended. An advisory was published by Intel: > https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00473.html > > I have tested the following cards successfully: > 7265, 8265, 9260, 9560 > > I cannot test any other devices myself right now; help with testing this > patch would be much appreciated. It is likely that all devices will work. > But I would still prefer to see test reports for the following cards before > committing this patch: > 7260, 3160, 3165, 3168, 8260 > > The 7260 and 3160 cards did not receive firmware updates from Intel. Which > could be good or bad news, depending on whether the devices are vulnerable > to fragattacks. The fragattacks test tools might shed light on this if you > are curious. I'd be interested to learn more if you have test results or > any other solid information regarding this. iwm0 at pci2 dev 0 function 0 "Intel AC 8260" rev 0x3a, msi iwm0: hw rev 0x200, fw ver 36.ca7b901d.0 Tested: - Downloading some big files - Doing stuff in a couple of SSH sessions - Roaming between some APs No regression...so far
Re: panic(9): set panicstr atomically
Alexander Bluhm wrote: > On Tue, May 25, 2021 at 04:15:26PM +0200, Mark Kettenis wrote: > > Wouldn't be too hard. But unless you're on a serial console, that > > will probably be more than a screenful of information, so not terribly > > useful. > > The most important things must fit on the first VGA screen. Then > user can make a photo. We ship products to customers who only > understand this simple step. And when the machine panics, the admin > might also panic. Matches my experience. It will be very difficult to show state of multiple cpus on a small screen. On the other hand, only showing the first-panic can hide so much. These garbled reports are actually highlighting a not-simple case. There is big information in the garble.
Re: panic(9): set panicstr atomically
On Tue, May 25, 2021 at 04:15:26PM +0200, Mark Kettenis wrote: > Wouldn't be too hard. But unless you're on a serial console, that > will probably be more than a screenful of information, so not terribly > useful. The most important things must fit on the first VGA screen. Then user can make a photo. We ship products to customers who only understand this simple step. And when the machine panics, the admin might also panic. bluhm
Re: panic(9): set panicstr atomically
> From: Scott Cheloha > Date: Tue, 25 May 2021 08:42:05 -0500 > > > On May 25, 2021, at 08:20, Theo de Raadt wrote: > > > > Scott Cheloha wrote: > > > >>> On Mon, May 24, 2021 at 10:12:53PM -0500, Scott Cheloha wrote: > >>> On Sat, May 22, 2021 at 01:35:53AM +0200, Alexander Bluhm wrote: > On Fri, May 21, 2021 at 02:00:54PM -0500, Scott Cheloha wrote: > > Given all of this, would it be better if secondary CPUs spin in > > panic(9) instead of trying to print anything? > > The panic code should be as primitive as possible. The garbled > output also tells me something. Two CPUs are failing simultaneosly. > Please don't suppress that information. > > The crash is the problem, not the ugly printing. > >>> > >>> I get where you're coming from in principle (simpler is better) but I > >>> think you're prioritizing a minor concern over the bigger picture. > >> > >> To be perfectly clear, I'm not talking about the garbled printing > >> anymore. I'm talking about *all* the code that can run from panic(). > >> There is a lot of code. I think it would be better if we prevented > >> multiple CPUs from running that code simultaneously by having > >> secondary CPUs spin in panic() as visa@ suggested. > > > > I think that is incorrect. There are no "secondary CPUs", there are > > only "not the first cpu to enter panic". > > Right, exactly. Not sure what else to call > them. We are talking about the same thing. > > > If the 2nd cpu to enter panic actually has a more relevant panic, now > > it will be missed. > > I said we could keep the printing. I even > kept the printing in my latest patch. So I > don't know what you're arguing against > here. So the big risk with your diff is that you may end up spinning with interrupts disabled at which point that CPU is dead. If you try to switch to it at that point, you'll lock up the machine. > > Most of our users don't do the ddbcpu trace dance. So I expect the > > reports we get as a result of your change will be less enlightening. > > This sounds like an argument for making the > "ddbcpu trace dance" easier. Or automating > it entirely. > > Personally I find it extremely annoying to flip > from CPU to CPU to print all the traces. It > only gets more annoying as the core count > grows. > > How hard would it be to implement > "show all trace" or something equivalent > to simplify crash reporting for the end user? Wouldn't be too hard. But unless you're on a serial console, that will probably be more than a screenful of information, so not terribly useful.
Re: panic(9): set panicstr atomically
Scott Cheloha wrote:
> > If the 2nd cpu to enter panic actually has a more relevant panic, now
> > it will be missed.
>
> I said we could keep the printing. I even
> kept the printing in my latest patch. So I
> don't know what you're arguing against
> here.
You start spinning cpus inside panic. I don't know if that breaks
the ddbcpu experience.
> Personally I find it extremely annoying to flip
> from CPU to CPU to print all the traces. It
> only gets more annoying as the core count
> grows.
>
> How hard would it be to implement
> "show all trace" or something equivalent
> to simplify crash reporting for the end user?
If there are too many lines if output, things may scroll out of view.
Excessively verbose context can be difficult to capture for reporting
purposes.
Here is a completely untested draft that would create a per-cpu panic
buffer, tries to make the top of of panic() operate on less global state,
and adjusts ddb "show panic" to show all the known panics. This isn't
tested, and the db_show_panic_cmd change is half baked.
Index: arch/amd64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.137
diff -u -p -u -r1.137 cpu.h
--- arch/amd64/include/cpu.h3 Jun 2020 06:54:04 - 1.137
+++ arch/amd64/include/cpu.h25 May 2021 13:25:13 -
@@ -208,6 +208,8 @@ struct cpu_info {
struct vmxon_region *ci_vmxon_region;
int64_t ci_tsc_skew;/* counter skew vs cpu0 */
+
+ charci_panicbuf[512];
};
#define CPUF_BSP 0x0001 /* CPU is the original BSP */
Index: kern/subr_prf.c
===
RCS file: /cvs/src/sys/kern/subr_prf.c,v
retrieving revision 1.103
diff -u -p -u -r1.103 subr_prf.c
--- kern/subr_prf.c 16 May 2021 15:10:20 - 1.103
+++ kern/subr_prf.c 25 May 2021 13:31:30 -
@@ -194,20 +194,19 @@ panic(const char *fmt, ...)
printf_flags |= TOCONS;
bootopt = RB_AUTOBOOT | RB_DUMP;
+
va_start(ap, fmt);
+ vsnprintf(curcpu()->ci_panicbuf, sizeof(curcpu()->ci_panicbuf), fmt,
ap);
+ va_end(ap);
+
if (panicstr)
bootopt |= RB_NOSYNC;
else {
- vsnprintf(panicbuf, sizeof panicbuf, fmt, ap);
panicstr = panicbuf;
+ strlcpy(panicbuf, curcpu()->ci_panicbuf, sizeof panicbuf);
}
- va_end(ap);
- printf("panic: ");
- va_start(ap, fmt);
- vprintf(fmt, ap);
- printf("\n");
- va_end(ap);
+ printf("panic: %s\n", curcpu()->ci_panicbuf);
#ifdef DDB
if (db_panic)
Index: ddb/db_command.c
===
RCS file: /cvs/src/sys/ddb/db_command.c,v
retrieving revision 1.90
diff -u -p -u -r1.90 db_command.c
--- ddb/db_command.c26 Oct 2020 18:53:20 - 1.90
+++ ddb/db_command.c25 May 2021 13:39:11 -
@@ -468,14 +468,23 @@ db_nfsnode_print_cmd(db_expr_t addr, int
void
db_show_panic_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
{
- if (panicstr)
+ if (panicstr) {
+#ifdef MULTIPROCESSOR
+ int i;
+
+ for (i = 0; i < MAXCPUS; i++) {
+ if (cpu_info[i] != NULL && curcpu()->ci_panicbuf[0])
+ db_printf("%d: %s\n", CPU_INFO_UNIT(curcpu()),
+ curcpu()->ci_panicbuf);
+ }
+#else
db_printf("%s\n", panicstr);
- else if (faultstr) {
+#endif
+ } else if (faultstr) {
db_printf("kernel page fault\n");
db_printf("%s\n", faultstr);
db_stack_trace_print(addr, have_addr, 1, modif, db_printf);
- }
- else
+ } else
db_printf("the kernel did not panic\n");/* yet */
}
iwm(4): use new firmware images with fragattack fixes
This patch allows iwm(4) to use new firmware images which are part
of the iwm-20210512 firmware package, available via fw_update (you
need to run fw_update *before* booting with this patch).
The new firmware images were published right after the fragattacks
embargo period ended. An advisory was published by Intel:
https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00473.html
I have tested the following cards successfully:
7265, 8265, 9260, 9560
I cannot test any other devices myself right now; help with testing this
patch would be much appreciated. It is likely that all devices will work.
But I would still prefer to see test reports for the following cards before
committing this patch:
7260, 3160, 3165, 3168, 8260
The 7260 and 3160 cards did not receive firmware updates from Intel. Which
could be good or bad news, depending on whether the devices are vulnerable
to fragattacks. The fragattacks test tools might shed light on this if you
are curious. I'd be interested to learn more if you have test results or
any other solid information regarding this.
diff refs/heads/master refs/heads/iwmfw
blob - 4b502468fea796f103fb84237146879e8c4df267
blob + be7f5b6c79866bc2e961c76433b350c60f748512
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -253,8 +253,11 @@ intiwm_firmware_store_section(struct iwm_softc *,
enu
uint8_t *, size_t);
intiwm_set_default_calib(struct iwm_softc *, const void *);
void iwm_fw_info_free(struct iwm_fw_info *);
+void iwm_fw_version_str(char *, size_t, uint32_t, uint32_t, uint32_t);
intiwm_read_firmware(struct iwm_softc *, enum iwm_ucode_type);
+uint32_t iwm_read_prph_unlocked(struct iwm_softc *, uint32_t);
uint32_t iwm_read_prph(struct iwm_softc *, uint32_t);
+void iwm_write_prph_unlocked(struct iwm_softc *, uint32_t, uint32_t);
void iwm_write_prph(struct iwm_softc *, uint32_t, uint32_t);
intiwm_read_mem(struct iwm_softc *, uint32_t, void *, int);
intiwm_write_mem(struct iwm_softc *, uint32_t, const void *, int);
@@ -292,6 +295,7 @@ voidiwm_apm_stop(struct iwm_softc *);
intiwm_allow_mcast(struct iwm_softc *);
void iwm_init_msix_hw(struct iwm_softc *);
void iwm_conf_msix_hw(struct iwm_softc *, int);
+intiwm_clear_persistence_bit(struct iwm_softc *);
intiwm_start_hw(struct iwm_softc *);
void iwm_stop_device(struct iwm_softc *);
void iwm_nic_config(struct iwm_softc *);
@@ -402,6 +406,8 @@ voidiwm_rx_compressed_ba(struct iwm_softc *, struct
i
void iwm_rx_bmiss(struct iwm_softc *, struct iwm_rx_packet *,
struct iwm_rx_data *);
intiwm_binding_cmd(struct iwm_softc *, struct iwm_node *, uint32_t);
+intiwm_phy_ctxt_cmd_uhb(struct iwm_softc *, struct iwm_phy_ctxt *, uint8_t,
+ uint8_t, uint32_t, uint32_t);
void iwm_phy_ctxt_cmd_hdr(struct iwm_softc *, struct iwm_phy_ctxt *,
struct iwm_phy_context_cmd *, uint32_t, uint32_t);
void iwm_phy_ctxt_cmd_data(struct iwm_softc *, struct iwm_phy_context_cmd *,
@@ -454,6 +460,7 @@ int iwm_fill_probe_req(struct iwm_softc *, struct iwm_
intiwm_lmac_scan(struct iwm_softc *, int);
intiwm_config_umac_scan(struct iwm_softc *);
intiwm_umac_scan(struct iwm_softc *, int);
+void iwm_mcc_update(struct iwm_softc *, struct iwm_mcc_chub_notif *);
uint8_tiwm_ridx2rate(struct ieee80211_rateset *, int);
intiwm_rval2ridx(int);
void iwm_ack_rates(struct iwm_softc *, struct iwm_node *, int *, int *);
@@ -470,6 +477,8 @@ int iwm_bgscan(struct ieee80211com *);
intiwm_umac_scan_abort(struct iwm_softc *);
intiwm_lmac_scan_abort(struct iwm_softc *);
intiwm_scan_abort(struct iwm_softc *);
+intiwm_phy_ctxt_update(struct iwm_softc *, struct iwm_phy_ctxt *,
+ struct ieee80211_channel *, uint8_t, uint8_t, uint32_t);
intiwm_auth(struct iwm_softc *);
intiwm_deauth(struct iwm_softc *);
intiwm_assoc(struct iwm_softc *);
@@ -495,7 +504,9 @@ voidiwm_fill_sf_command(struct iwm_softc *, struct
iw
struct ieee80211_node *);
intiwm_sf_config(struct iwm_softc *, int);
intiwm_send_bt_init_conf(struct iwm_softc *);
+intiwm_send_soc_conf(struct iwm_softc *);
intiwm_send_update_mcc_cmd(struct iwm_softc *, const char *);
+intiwm_send_temp_report_ths_cmd(struct iwm_softc *);
void iwm_tt_tx_backoff(struct iwm_softc *, uint32_t);
void iwm_free_fw_paging(struct iwm_softc *);
intiwm_save_fw_paging(struct iwm_softc *, const struct iwm_fw_sects *);
@@ -546,6 +557,21 @@ intiwm_resume(struct iwm_softc *);
void iwm_radiotap_attach(struct iwm_softc *);
#endif
+uint8_t
+iwm_lookup_cmd_ver(struct iwm_softc *sc, uint8_t grp, uint8_t cmd)
+{
+ const struct iwm_fw_cmd_version *entry;
+ int i;
+
+ for (i = 0; i < sc->n_cmd_versions; i++) {
+ entry = >cmd_versions[i];
+ if (entry->group == grp && entry->cmd == cmd)
+ return
Re: panic(9): set panicstr atomically
> On May 25, 2021, at 08:20, Theo de Raadt wrote: > > Scott Cheloha wrote: > >>> On Mon, May 24, 2021 at 10:12:53PM -0500, Scott Cheloha wrote: >>> On Sat, May 22, 2021 at 01:35:53AM +0200, Alexander Bluhm wrote: On Fri, May 21, 2021 at 02:00:54PM -0500, Scott Cheloha wrote: > Given all of this, would it be better if secondary CPUs spin in > panic(9) instead of trying to print anything? The panic code should be as primitive as possible. The garbled output also tells me something. Two CPUs are failing simultaneosly. Please don't suppress that information. The crash is the problem, not the ugly printing. >>> >>> I get where you're coming from in principle (simpler is better) but I >>> think you're prioritizing a minor concern over the bigger picture. >> >> To be perfectly clear, I'm not talking about the garbled printing >> anymore. I'm talking about *all* the code that can run from panic(). >> There is a lot of code. I think it would be better if we prevented >> multiple CPUs from running that code simultaneously by having >> secondary CPUs spin in panic() as visa@ suggested. > > I think that is incorrect. There are no "secondary CPUs", there are > only "not the first cpu to enter panic". Right, exactly. Not sure what else to call them. We are talking about the same thing. > If the 2nd cpu to enter panic actually has a more relevant panic, now > it will be missed. I said we could keep the printing. I even kept the printing in my latest patch. So I don't know what you're arguing against here. > Most of our users don't do the ddbcpu trace dance. So I expect the > reports we get as a result of your change will be less enlightening. This sounds like an argument for making the "ddbcpu trace dance" easier. Or automating it entirely. Personally I find it extremely annoying to flip from CPU to CPU to print all the traces. It only gets more annoying as the core count grows. How hard would it be to implement "show all trace" or something equivalent to simplify crash reporting for the end user?
Re: ld.so: program headers: do not rely on DYNAMIC coming before GNU_RELRO
On Tue, May 25, 2021 at 12:00:21AM -0900, Philip Guenther wrote:
> On Mon, May 24, 2021 at 4:59 AM Klemens Nanni wrote:
>
> > When tinkering with ld.so crashes due to file corruption the other day
> > I tested a few changes but did not want to replace /usr/libexec/ld.so
> > and since recompiling executable to change their interpreter is not
> > always an option, I went for https://github.com/NixOS/patchelf which
> > allows me to manipulate executables in place.
> >
> > The tool works just fine and as a byproduct rearanges program headers;
> > that in itself is fine except our run-time link-editor is not happy with
> > such valid executables:
> >
> >
> > ELF mandates nothing but the file header be at a fixed location, hence
> > ld.so(1) must not assume any specific order for headers, segments, etc.
> >
>
> (Not quite: it does place ordering requirements in some specific cases,
> such as that PT_INTERP and PT_PHDR, if present, must appear before any
> PT_LOAD segments in the program header, and that PT_LOAD segments must
> appear in p_vaddr order.)
Thanks for clarifying on that. I've rechecked and this goes in line
with ELF Version 1.1 Section 2-2 "Program Loading and Dynamic Linking",
"Segment Types":
PT_LOAD[...] Loadable segment entries in the program header table
appear in ascending order, sorted on the p_vaddr member.
PT_INTERP [...] If it is present, it must precede any loadable segment
entry. [...]
> Looping over the program header table to parse segment headers,
> > _dl_boot() creates the executable object upon DYNAMIC and expects it to
> > be set upon GNU_RELRO, resulting in a NULL dereference iff that order is
> > reversed.
> >
> > Store relocation bits in temporary variables and update the executable
> > object once all segment headers are parsed to lift this dependency.
> >
> > Under __mips__ _dl_boot() later on uses the same temporary variable, so
> > move nothing but the declaration out of MI code so as to not alter the
> > MD code's logic/behaviour.
> >
> >
> > This fix is needed for the following work on OpenBSD:
> >
> > $ patchelf --set-interpreter $PWD/obj/ld.so ./my-ldso-test
> > $ readelf -l ./my-ldso-test | grep interpreter
> > [Requesting program interpreter: /usr/src/libexec/
> > ld.so/obj/ld.so]
> > $ ./my-ldso-test
> > it works!
> >
> > amd64 and arm64 regress is happy and all my patched executables work
> > with this.
> >
> > Feedback? Objections? OK?
> >
> > diff d7231fb4fb547dd287a884c56ae7c8b10f9145fe
> > f023dbe355bef379d55eb93eddbb2702559d5bdb
> > blob - 18bd30af759bffbc4e3fbfee9ffc29906f0d1bb0
> > blob + b66dbb169aad9afffa1283d480ad9276aff9072a
> > --- libexec/ld.so/loader.c
> > +++ libexec/ld.so/loader.c
> > @@ -464,6 +464,7 @@ _dl_boot(const char **argv, char **envp, const long dy
> > int failed;
> > struct dep_node *n;
> > Elf_Addr minva, maxva, exe_loff, exec_end, cur_exec_end;
> > + Elf_Addr relro_addr = 0, relro_size = 0;
> > Elf_Phdr *ptls = NULL;
> > int align;
> >
> > @@ -552,8 +553,8 @@ _dl_boot(const char **argv, char **envp, const long dy
> > ptls = phdp;
> > break;
> > case PT_GNU_RELRO:
> > - exe_obj->relro_addr = phdp->p_vaddr + exe_loff;
> > - exe_obj->relro_size = phdp->p_memsz;
> > + relro_addr = phdp->p_vaddr + exe_loff;
> > + relro_size = phdp->p_memsz;
> > break;
> > }
> > phdp++;
> > @@ -561,6 +562,8 @@ _dl_boot(const char **argv, char **envp, const long dy
> > exe_obj->load_list = load_list;
> > exe_obj->obj_flags |= DF_1_GLOBAL;
> > exe_obj->load_size = maxva - minva;
> > + exe_obj->relro_addr = relro_addr;
> > + exe_obj->relro_size = relro_size;
> > _dl_set_sod(exe_obj->load_name, _obj->sod);
> >
> > #ifdef __i386__
> > @@ -638,7 +641,7 @@ _dl_boot(const char **argv, char **envp, const long dy
> > debug_map->r_ldbase = dyn_loff;
> > _dl_debug_map = debug_map;
> > #ifdef __mips__
> > - Elf_Addr relro_addr = exe_obj->relro_addr;
> > + relro_addr = exe_obj->relro_addr;
> > if (dynp->d_tag == DT_DEBUG &&
> > ((Elf_Addr)map_link + sizeof(*map_link) <= relro_addr
> > ||
> > (Elf_Addr)map_link >= relro_addr +
> > exe_obj->relro_size)) {
> >
> >
> ok guenther@
>
> (This still assumes PT_GNU_RELRO comes after PT_PHDR, for exe_loff, but I
> suspect even patchelf wouldn't screw with that.)
Yes, not just PT_GNU_RELRO but PT_DYNAMIC and PT_LOAD as well due to
`exe_loff' in our code.
PT_LOAD *must* come after PT_PHDR according to the above mentioned, on
PT_DYNAMIC however I couldn't find any such requirement in the spec.
Re: panic(9): set panicstr atomically
Scott Cheloha wrote: > On Mon, May 24, 2021 at 10:12:53PM -0500, Scott Cheloha wrote: > > On Sat, May 22, 2021 at 01:35:53AM +0200, Alexander Bluhm wrote: > > > On Fri, May 21, 2021 at 02:00:54PM -0500, Scott Cheloha wrote: > > > > Given all of this, would it be better if secondary CPUs spin in > > > > panic(9) instead of trying to print anything? > > > > > > The panic code should be as primitive as possible. The garbled > > > output also tells me something. Two CPUs are failing simultaneosly. > > > Please don't suppress that information. > > > > > > The crash is the problem, not the ugly printing. > > > > I get where you're coming from in principle (simpler is better) but I > > think you're prioritizing a minor concern over the bigger picture. > > To be perfectly clear, I'm not talking about the garbled printing > anymore. I'm talking about *all* the code that can run from panic(). > There is a lot of code. I think it would be better if we prevented > multiple CPUs from running that code simultaneously by having > secondary CPUs spin in panic() as visa@ suggested. I think that is incorrect. There are no "secondary CPUs", there are only "not the first cpu to enter panic". If the 2nd cpu to enter panic actually has a more relevant panic, now it will be missed. Most of our users don't do the ddbcpu trace dance. So I expect the reports we get as a result of your change will be less enlightening.
Re: bgpd extend capability support (add-path, enhanced rr)
On Tue, May 18, 2021 at 05:35:01PM +0200, Claudio Jeker wrote:
> bgpd(8) will soon support ADD-PATH (RFC7911) and enhanced route refresh
> (RFC7313). This is the frist step toward this.
> It add the capability parsers, extends the capability struct and adds the
> capability negotiation bits. The route refresh parser and generator are
> extended to support the BoRR and EoRR message and last but not least
> bgpctl is adjusted to print the new capabilities.
>
> Now since the system has no way of enabling the two new capabilities.
> bgpctl will only show if the peer sends the capability but there should
> be no other effect.
>
> The RDE bits for enahnced route refresh are almost ready, add-path will
> take a bit more since the RDE needs to grow an extra indirection.
>
> I decided to split this work up to simplify review. Lets see if this
> works :)
Since this does not change any behaviour I consider to commit this soon
unles someone objects.
--
:wq Claudio
Index: bgpctl/bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.267
diff -u -p -r1.267 bgpctl.c
--- bgpctl/bgpctl.c 3 May 2021 14:01:56 - 1.267
+++ bgpctl/bgpctl.c 18 May 2021 08:49:34 -
@@ -1351,6 +1351,13 @@ print_capability(u_int8_t capa_code, u_c
} else
printf("bad length");
break;
+ case CAPA_ADD_PATH:
+ printf("add-path capability");
+ /* XXX there is more needed here */
+ break;
+ case CAPA_ENHANCED_RR:
+ printf("enhanced route refresh capability");
+ break;
default:
printf("unknown capability %u length %u", capa_code, len);
break;
Index: bgpctl/output.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v
retrieving revision 1.16
diff -u -p -r1.16 output.c
--- bgpctl/output.c 26 Apr 2021 18:23:20 - 1.16
+++ bgpctl/output.c 18 May 2021 15:17:39 -
@@ -147,6 +147,34 @@ show_neighbor_capa_mp(struct capabilitie
}
static void
+show_neighbor_capa_add_path(struct capabilities *capa)
+{
+ const char *mode;
+ int comma;
+ u_int8_ti;
+
+ printf("Add-path: ");
+ for (i = 0, comma = 0; i < AID_MAX; i++) {
+ switch (capa->add_path[i]) {
+ case 0:
+ default:
+ continue;
+ case CAPA_AP_RECV:
+ mode = "recv";
+ break;
+ case CAPA_AP_SEND:
+ mode = "send";
+ break;
+ case CAPA_AP_BIDIR:
+ mode = "bidir";
+ }
+ printf("%s%s %s", comma ? ", " : "", aid2str(i), mode);
+ comma = 1;
+ }
+ printf("\n");
+}
+
+static void
show_neighbor_capa_restart(struct capabilities *capa)
{
int comma;
@@ -202,6 +230,13 @@ show_neighbor_msgstats(struct peer *p)
p->stats.prefix_sent_withdraw, p->stats.prefix_rcvd_withdraw);
printf(" %-15s %10llu %10llu\n", "End-of-Rib",
p->stats.prefix_sent_eor, p->stats.prefix_rcvd_eor);
+ printf(" Route Refresh statistics:\n");
+ printf(" %-15s %10llu %10llu\n", "Request",
+ p->stats.refresh_sent_req, p->stats.refresh_rcvd_req);
+ printf(" %-15s %10llu %10llu\n", "Begin-of-RR",
+ p->stats.refresh_sent_borr, p->stats.refresh_rcvd_borr);
+ printf(" %-15s %10llu %10llu\n", "End-of-RR",
+ p->stats.refresh_sent_eorr, p->stats.refresh_rcvd_eorr);
}
static void
@@ -210,7 +245,7 @@ show_neighbor_full(struct peer *p, struc
const char *errstr;
struct in_addr ina;
char*s;
- int hascapamp = 0;
+ int hascapamp, hascapaap;
u_int8_t i;
if ((p->conf.remote_addr.aid == AID_INET &&
@@ -279,35 +314,57 @@ show_neighbor_full(struct peer *p, struc
fmt_monotime(p->stats.last_read),
p->holdtime, p->holdtime/3);
printf(" Last write %s\n", fmt_monotime(p->stats.last_write));
- for (i = 0; i < AID_MAX; i++)
+
+ hascapamp = 0;
+ hascapaap = 0;
+ for (i = AID_MIN; i < AID_MAX; i++) {
if (p->capa.peer.mp[i])
hascapamp = 1;
- if (hascapamp || p->capa.peer.refresh ||
- p->capa.peer.grestart.restart || p->capa.peer.as4byte) {
+ if (p->capa.peer.add_path[i])
+ hascapaap = 1;
+ }
+ if (hascapamp || hascapaap || p->capa.peer.grestart.restart ||
+ p->capa.peer.refresh || p->capa.peer.enhanced_rr ||
+ p->capa.peer.as4byte) {
Re: bgpd upgrade to RFC6793
On Tue, May 18, 2021 at 12:48:06PM +0200, Claudio Jeker wrote:
> Our four-byte AS support dates back to the days of the original draft.
> Since then a new RFC 6793 got released that adjusted the error handling a
> bit. RFC 6793 just treats any error on AS4_PATH attribute with attribute
> drop with the hope that the AS_PATH is better.
>
> The reason is a bit questionable:
>Given that the two-octet AS numbers dominate during the transition
>and are carried in the AS_PATH attribute by an OLD BGP speaker, in
>this document the "attribute discard" approach is chosen to handle a
>malformed AS4_PATH attribute.
> My IPv4 feed has 41148 of 71059 or 57% two-octet AS numbers. I would not
> call that "dominate" (maybe the transition period is over...)
>
> Anyway, lets follow the standard
Ping
--
:wq Claudio
Index: bgpd.8
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v
retrieving revision 1.66
diff -u -p -r1.66 bgpd.8
--- bgpd.8 27 Apr 2021 11:34:58 - 1.66
+++ bgpd.8 18 May 2021 10:04:47 -
@@ -317,14 +317,6 @@ has been started.
.Re
.Pp
.Rs
-.%A Q. Vohra
-.%A E. Chen
-.%D May 2007
-.%R RFC 4893
-.%T BGP Support for Four-octet AS Number Space
-.Re
-.Pp
-.Rs
.%A V. Gill
.%A J. Heasley
.%A D. Meyer
@@ -349,6 +341,14 @@ has been started.
.%D June 2011
.%R RFC 6286
.%T Autonomous-System-Wide Unique BGP Identifier for BGP-4
+.Re
+.Pp
+.Rs
+.%A Q. Vohra
+.%A E. Chen
+.%D Dec 2012
+.%R RFC 6793
+.%T BGP Support for Four-Octet Autonomous System (AS) Number Space
.Re
.Pp
.Rs
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.520
diff -u -p -r1.520 rde.c
--- rde.c 6 May 2021 09:18:54 - 1.520
+++ rde.c 18 May 2021 10:29:15 -
@@ -1918,27 +1918,11 @@ bad_flags:
goto bad_flags;
if ((error = aspath_verify(p, attr_len, 1,
rde_no_as_set(peer))) != 0) {
- /*
-* XXX RFC does not specify how to handle errors.
-* XXX Instead of dropping the session because of a
-* XXX bad path just mark the full update as having
-* XXX a parse error which makes the update no longer
-* XXX eligible and will not be considered for routing
-* XXX or redistribution.
-* XXX We follow draft-ietf-idr-optional-transitive
-* XXX by looking at the partial bit.
-* XXX Consider soft errors similar to a partial attr.
-*/
- if (flags & ATTR_PARTIAL || error == AS_ERR_SOFT) {
- a->flags |= F_ATTR_PARSE_ERR;
- log_peer_warnx(>conf, "bad AS4_PATH, "
- "path invalidated and prefix withdrawn");
- goto optattr;
- } else {
- rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH,
- NULL, 0);
- return (-1);
- }
+ /* As per RFC6793 use "attribute discard" here. */
+ log_peer_warnx(>conf, "bad AS4_PATH, "
+ "attribute discarded");
+ plen += attr_len;
+ break;
}
a->flags |= F_ATTR_AS4BYTE_NEW;
goto optattr;
Re: ld.so: program headers: do not rely on DYNAMIC coming before GNU_RELRO
On Mon, May 24, 2021 at 4:59 AM Klemens Nanni wrote:
> When tinkering with ld.so crashes due to file corruption the other day
> I tested a few changes but did not want to replace /usr/libexec/ld.so
> and since recompiling executable to change their interpreter is not
> always an option, I went for https://github.com/NixOS/patchelf which
> allows me to manipulate executables in place.
>
> The tool works just fine and as a byproduct rearanges program headers;
> that in itself is fine except our run-time link-editor is not happy with
> such valid executables:
>
>
> ELF mandates nothing but the file header be at a fixed location, hence
> ld.so(1) must not assume any specific order for headers, segments, etc.
>
(Not quite: it does place ordering requirements in some specific cases,
such as that PT_INTERP and PT_PHDR, if present, must appear before any
PT_LOAD segments in the program header, and that PT_LOAD segments must
appear in p_vaddr order.)
Looping over the program header table to parse segment headers,
> _dl_boot() creates the executable object upon DYNAMIC and expects it to
> be set upon GNU_RELRO, resulting in a NULL dereference iff that order is
> reversed.
>
> Store relocation bits in temporary variables and update the executable
> object once all segment headers are parsed to lift this dependency.
>
> Under __mips__ _dl_boot() later on uses the same temporary variable, so
> move nothing but the declaration out of MI code so as to not alter the
> MD code's logic/behaviour.
>
>
> This fix is needed for the following work on OpenBSD:
>
> $ patchelf --set-interpreter $PWD/obj/ld.so ./my-ldso-test
> $ readelf -l ./my-ldso-test | grep interpreter
> [Requesting program interpreter: /usr/src/libexec/
> ld.so/obj/ld.so]
> $ ./my-ldso-test
> it works!
>
> amd64 and arm64 regress is happy and all my patched executables work
> with this.
>
> Feedback? Objections? OK?
>
> diff d7231fb4fb547dd287a884c56ae7c8b10f9145fe
> f023dbe355bef379d55eb93eddbb2702559d5bdb
> blob - 18bd30af759bffbc4e3fbfee9ffc29906f0d1bb0
> blob + b66dbb169aad9afffa1283d480ad9276aff9072a
> --- libexec/ld.so/loader.c
> +++ libexec/ld.so/loader.c
> @@ -464,6 +464,7 @@ _dl_boot(const char **argv, char **envp, const long dy
> int failed;
> struct dep_node *n;
> Elf_Addr minva, maxva, exe_loff, exec_end, cur_exec_end;
> + Elf_Addr relro_addr = 0, relro_size = 0;
> Elf_Phdr *ptls = NULL;
> int align;
>
> @@ -552,8 +553,8 @@ _dl_boot(const char **argv, char **envp, const long dy
> ptls = phdp;
> break;
> case PT_GNU_RELRO:
> - exe_obj->relro_addr = phdp->p_vaddr + exe_loff;
> - exe_obj->relro_size = phdp->p_memsz;
> + relro_addr = phdp->p_vaddr + exe_loff;
> + relro_size = phdp->p_memsz;
> break;
> }
> phdp++;
> @@ -561,6 +562,8 @@ _dl_boot(const char **argv, char **envp, const long dy
> exe_obj->load_list = load_list;
> exe_obj->obj_flags |= DF_1_GLOBAL;
> exe_obj->load_size = maxva - minva;
> + exe_obj->relro_addr = relro_addr;
> + exe_obj->relro_size = relro_size;
> _dl_set_sod(exe_obj->load_name, _obj->sod);
>
> #ifdef __i386__
> @@ -638,7 +641,7 @@ _dl_boot(const char **argv, char **envp, const long dy
> debug_map->r_ldbase = dyn_loff;
> _dl_debug_map = debug_map;
> #ifdef __mips__
> - Elf_Addr relro_addr = exe_obj->relro_addr;
> + relro_addr = exe_obj->relro_addr;
> if (dynp->d_tag == DT_DEBUG &&
> ((Elf_Addr)map_link + sizeof(*map_link) <= relro_addr
> ||
> (Elf_Addr)map_link >= relro_addr +
> exe_obj->relro_size)) {
>
>
ok guenther@
(This still assumes PT_GNU_RELRO comes after PT_PHDR, for exe_loff, but I
suspect even patchelf wouldn't screw with that.)
Re: Add f_modify and f_process callbacks to socket filterops
On 20/05/21(Thu) 14:16, Visa Hankala wrote:
> On Thu, May 20, 2021 at 11:35:32AM +0200, Martin Pieuchot wrote:
> > On 18/05/21(Tue) 14:22, Visa Hankala wrote:
> > > This diff adds f_modify and f_process callbacks to socket event filters.
> > > As a result, socket events are handled using the non-legacy paths in
> > > filter_modify() and filter_process() of kern_event.c This a step toward
> > > MP-safety. However, everything still runs under the kernel lock.
> > >
> > > The change has three intended effects:
> > >
> > > * Socket events are handled without raising the system priority level.
> > > This makes the activity observable with btrace(8).
> > >
> > > * kqueue itself no longer calls f_event of socket filterops, which
> > > allows replacing the conditional, NOTE_SUBMIT-based locking with
> > > a fixed call pattern.
> >
> > I love this.
> >
> > > * The state of a socket event is now always rechecked before delivery
> > > to user. Before, the recheck was skipped if the event was registered
> > > with EV_ONESHOT.
> >
> > To me this sounds sane. I can't think of a way to rely on the current
> > behavior. However if there's an easy way to split these changes in two
> > commits, I'd prefer to stay on the safe side.
>
> Below is an updated diff that preserves the current EV_ONESHOT
> behaviour. I have just adapted a part of the compatibility logic
> from function filter_process().
>
> When f_process is given a non-NULL kev argument, it is known that
> the callback is invoked from kqueue_scan(). If kev is NULL,
> kqueue_register() is checking if the knote should be activated and
> there is no intent to deliver the event right now.
ok mpi@
> Index: kern/uipc_socket.c
> ===
> RCS file: src/sys/kern/uipc_socket.c,v
> retrieving revision 1.261
> diff -u -p -r1.261 uipc_socket.c
> --- kern/uipc_socket.c13 May 2021 19:43:11 - 1.261
> +++ kern/uipc_socket.c20 May 2021 14:01:18 -
> @@ -70,15 +70,26 @@ void sorflush(struct socket *);
>
> void filt_sordetach(struct knote *kn);
> int filt_soread(struct knote *kn, long hint);
> +int filt_soreadmodify(struct kevent *kev, struct knote *kn);
> +int filt_soreadprocess(struct knote *kn, struct kevent *kev);
> +int filt_soread_common(struct knote *kn, struct socket *so);
> void filt_sowdetach(struct knote *kn);
> int filt_sowrite(struct knote *kn, long hint);
> +int filt_sowritemodify(struct kevent *kev, struct knote *kn);
> +int filt_sowriteprocess(struct knote *kn, struct kevent *kev);
> +int filt_sowrite_common(struct knote *kn, struct socket *so);
> int filt_solisten(struct knote *kn, long hint);
> +int filt_solistenmodify(struct kevent *kev, struct knote *kn);
> +int filt_solistenprocess(struct knote *kn, struct kevent *kev);
> +int filt_solisten_common(struct knote *kn, struct socket *so);
>
> const struct filterops solisten_filtops = {
> .f_flags= FILTEROP_ISFD,
> .f_attach = NULL,
> .f_detach = filt_sordetach,
> .f_event= filt_solisten,
> + .f_modify = filt_solistenmodify,
> + .f_process = filt_solistenprocess,
> };
>
> const struct filterops soread_filtops = {
> @@ -86,6 +97,8 @@ const struct filterops soread_filtops =
> .f_attach = NULL,
> .f_detach = filt_sordetach,
> .f_event= filt_soread,
> + .f_modify = filt_soreadmodify,
> + .f_process = filt_soreadprocess,
> };
>
> const struct filterops sowrite_filtops = {
> @@ -93,6 +106,8 @@ const struct filterops sowrite_filtops =
> .f_attach = NULL,
> .f_detach = filt_sowdetach,
> .f_event= filt_sowrite,
> + .f_modify = filt_sowritemodify,
> + .f_process = filt_sowriteprocess,
> };
>
> const struct filterops soexcept_filtops = {
> @@ -100,6 +115,8 @@ const struct filterops soexcept_filtops
> .f_attach = NULL,
> .f_detach = filt_sordetach,
> .f_event= filt_soread,
> + .f_modify = filt_soreadmodify,
> + .f_process = filt_soreadprocess,
> };
>
> #ifndef SOMINCONN
> @@ -2056,13 +2073,12 @@ filt_sordetach(struct knote *kn)
> }
>
> int
> -filt_soread(struct knote *kn, long hint)
> +filt_soread_common(struct knote *kn, struct socket *so)
> {
> - struct socket *so = kn->kn_fp->f_data;
> - int s, rv = 0;
> + int rv = 0;
> +
> + soassertlocked(so);
>
> - if ((hint & NOTE_SUBMIT) == 0)
> - s = solock(so);
> kn->kn_data = so->so_rcv.sb_cc;
> #ifdef SOCKET_SPLICE
> if (isspliced(so)) {
> @@ -2090,12 +2106,50 @@ filt_soread(struct knote *kn, long hint)
> } else {
> rv = (kn->kn_data >= so->so_rcv.sb_lowat);
> }
> - if ((hint & NOTE_SUBMIT) == 0)
> - sounlock(so, s);
>
> return rv;
> }
>
> +int
> +filt_soread(struct knote *kn, long hint)
>
