Re: iked.conf.5: provide gre example
On Thu, Jul 16, 2020 at 03:02:25PM +0200, Klemens Nanni wrote: > On Thu, Jul 16, 2020 at 10:23:20AM +0100, Stuart Henderson wrote: > > On 2020/07/15 10:02, Theo de Raadt wrote: > > > It is extremely unwise to use DNS names at this level (or things which > > > look like DNS names). The same problems that pf has with DNS, are > > > present here. You really don't want people to get into this habit. > > > > Same in gre(4) config which needs addresses too. I agree. > Alright, using literal IPs. > > > > > +.Pp > > > > +This example encrypts a > > > > +.Xr gre 4 > > > > +tunnel from local machine A to peer D using FQDN-based public key > > > > +authentication. > > > > +.Ar transport > > > > +mode is used to avoid duplicate encapsulation of GRE; > > > > The inside encapsulation of IPsec tunnel mode is gif not gre, so it > > isn't duplicate gre encap. "transport mode is used to avoid double > > encapsulation" would do? > Right, I didn't mean "twice GRE" but rather "twice encap": your wording > is clearer, thanks. > > dstid omitted as requested by tobhe. Ping. Feedback? OK? Index: iked.conf.5 === RCS file: /cvs/src/sbin/iked/iked.conf.5,v retrieving revision 1.71 diff -u -p -r1.71 iked.conf.5 --- iked.conf.5 10 Jul 2020 21:23:47 - 1.71 +++ iked.conf.5 16 Jul 2020 12:59:13 - @@ -1014,6 +1014,19 @@ ikev2 "subnet" esp from 10.0.3.0/24 to 1 ikev2 esp from 10.0.5.0/30 to 10.0.5.4/30 peer 192.168.1.2 ikev2 esp from 10.0.5.8/30 to 10.0.5.12/30 peer 192.168.1.3 .Ed +.Pp +This example encrypts a +.Xr gre 4 +tunnel from local machine A (2001:db8::aa:1) to peer D (2001:db8::dd:4) based on +FQDN-based public key authentication; +.Ar transport +mode avoids double encapsulation: +.Bd -literal -offset indent +ikev2 transport \e + proto gre \e + from 2001:db8::aa:1 to 2001:db8::dd:4 \e + peer D.example.com +.Ed .Sh SEE ALSO .Xr enc 4 , .Xr ipsec 4 ,
Re: ftpd: ignore -l in LIST path
> On Jul 18, 2020, at 8:16 PM, Brian Brombacher wrote:
>
> Hello,
>
> Below is a patch that repairs ftpd LIST operation for at least Chrome browser.
> Discard the patch if the intention is to make clients change behavior. A bug
> report has already been filed with the browser. Edge browser works fine.
> Other browsers untested.
>
> Symptom in raw output in Chrome 83:
>
> ftpd: -l: No such file or directory
>
> Reproduce using Chrome 83:
>
> ftp:///?raw
>
> Cheers,
> Brian
>
>
> Index: popen.c
> ===
> RCS file: /home/brian/cvs/src/libexec/ftpd/popen.c,v
> retrieving revision 1.29
> diff -u -r1.29 popen.c
> --- popen.c15 Jan 2020 22:06:59 -1.29
> +++ popen.c18 Jul 2020 23:52:49 -
> @@ -76,7 +76,7 @@
>argv[argc++] = "--";
>
>/* glob that path */
> -if (path != NULL) {
> +if (path != NULL && strcmp(path, "-l") != 0) {
>glob_t gl;
>
>memset(, 0, sizeof(gl));
>
Update from Chromium team, looks like it’s a non-issue for this browser going
forward.
Updates:
Status: WontFix
Comment #4 on issue 1107143 by asa...@chromium.org: OpenBSD ftpd LIST
incompatible
https://bugs.chromium.org/p/chromium/issues/detail?id=1107143#c4
Thank you for the report. We are in the process of deprecating and removing FTP
support as described in https://www.chromestatus.com/features/6246151319715840 .
Cheers,
Brian
Re: pf: remove ptr_array from struct pf_ruleset
On Mon, Jul 20, 2020 at 01:14:00PM +0200, Alexandr Nedvedicky wrote:
> I took a closer look at your change and related area. Below is an alternate
> way to fix the bug you've found.
Thanks for bringing it up again, I forgot to reply earlier.
> there are few details worth to note:
>
> ptr_array matters to main ruleset only, because it is the only
> ruleset covered by MD5 sum for pfsync.
Correct, as is directly seen by the only caller pf_commit_rules():
819 /* Calculate checksum for the main ruleset */
820 if (rs == _main_ruleset) {
821 error = pf_setup_pfsync_matching(rs);
822 if (error != 0)
823 return (error);
824 }
> it looks like we should also recompute the MD5sum if we remove the rule
> from the main ruleset
Yes, but that is a separate bug, regardless of how we handle checksum
calculation and rule lookups.
My plan was to tackle this next, but reusing existing code instead:
After my initial diff, pf_setup_pfsync_matching() would merely update
the checksum, so we could rename the function to something more
appropiate and call it from pf_purge_rule() instead of duplicating it
there.
> my diff introduces a new member of pr_ruleset structure, which keeps the
> array size so we can pass it to free(9f) later.
I persued this first (as done in other free() size diffs) but eventually
opted for the removal of ptr_array for the sake of simplicity.
> I have no pfsync deployment readily available for testing. Will be glad
> if you can give my diff a try.
Works as expected in both regards: pfsync picks the right rule and the
checksum updates after rules expired.
Re: Use VGA text mode palette RGB values in rasops(9)
On Wed, Jul 15, 2020 at 10:26:12AM -0600, Theo de Raadt wrote:
> > So here is a new iteration taking feedback into account, using the
> > #if WS_DEFAULT_BG == WSCOL_WHITE check for clarity, and also switching
> > the foreground color of printed kernel messages to light cyan to improve
> > contrast and readability.
>
> I really dislike how two issues are being mixed into one diff.
Right. This was to allow easier testing by having only one diff to
apply, and that's why I wasn't explicitely asking for OKs.
So let's move forward with this, here is a diff addressing only the
color palette changes.
OK?
Index: sys/dev/rasops/rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.63
diff -u -p -r1.63 rasops.c
--- sys/dev/rasops/rasops.c 11 Jul 2020 15:02:52 - 1.63
+++ sys/dev/rasops/rasops.c 15 Jul 2020 14:55:45 -
@@ -47,7 +47,8 @@
/* ANSI colormap (R,G,B) */
-#defineNORMAL_BLACK0x00
+#if WS_DEFAULT_BG == WSCOL_WHITE
+#defineNORMAL_BLACK0x00/* Rasops palette */
#defineNORMAL_RED 0x7f
#defineNORMAL_GREEN0x007f00
#defineNORMAL_BROWN0x7f7f00
@@ -64,6 +65,25 @@
#defineHILITE_MAGENTA 0xff00ff
#defineHILITE_CYAN 0x00
#defineHILITE_WHITE0xff
+#else
+#defineNORMAL_BLACK0x00/* VGA text mode palette */
+#defineNORMAL_RED 0xaa
+#defineNORMAL_GREEN0x00aa00
+#defineNORMAL_BROWN0xaa5500
+#defineNORMAL_BLUE 0xaa
+#defineNORMAL_MAGENTA 0xaa00aa
+#defineNORMAL_CYAN 0x00
+#defineNORMAL_WHITE0xaa
+
+#defineHILITE_BLACK0x55
+#defineHILITE_RED 0xff
+#defineHILITE_GREEN0x55ff55
+#defineHILITE_BROWN0x55
+#defineHILITE_BLUE 0xff
+#defineHILITE_MAGENTA 0xff55ff
+#defineHILITE_CYAN 0x55
+#defineHILITE_WHITE0xff
+#endif
const u_char rasops_cmap[256 * 3] = {
#define_C(x) ((x) & 0xff) >> 16, ((x) & 0x00ff00) >> 8, ((x) &
0xff)
Allow the WSDISPLAYIO_GETSCREENTYPE ioctl on tty*cfg
Hi tech@,
Here is a diff to allow the WSDISPLAYIO_GETSCREENTYPE ioctl on the
tty*cfg device, passing it back to tty*0.
I need this to restore working defaults in wsfontload(8).
Comments? OK?
Index: sys/dev/wscons/wsdisplay.c
===
RCS file: /cvs/src/sys/dev/wscons/wsdisplay.c,v
retrieving revision 1.141
diff -u -p -r1.141 wsdisplay.c
--- sys/dev/wscons/wsdisplay.c 25 May 2020 09:55:49 - 1.141
+++ sys/dev/wscons/wsdisplay.c 20 Jul 2020 14:28:42 -
@@ -1046,10 +1046,15 @@ wsdisplayioctl(dev_t dev, u_long cmd, ca
#endif
if (ISWSDISPLAYCTL(dev)) {
- if (cmd != WSDISPLAYIO_GTYPE)
+ switch (cmd) {
+ case WSDISPLAYIO_GTYPE:
+ case WSDISPLAYIO_GETSCREENTYPE:
+ /* pass to the first screen */
+ dev = makedev(major(dev), WSDISPLAYMINOR(unit, 0));
+ break;
+ default:
return (wsdisplay_cfg_ioctl(sc, cmd, data, flag, p));
- /* pass WSDISPLAYIO_GTYPE to the first screen */
- dev = makedev(major(dev), WSDISPLAYMINOR(unit, 0));
+ }
}
if (WSDISPLAYSCREEN(dev) >= WSDISPLAY_MAXSCREEN)
Re: switch default MANPAGER from more(1) to less(1)
Hi, ropers wrote on Mon, Jul 20, 2020 at 05:54:46AM +0100: > Ah, I see where you're coming from, Ingo. You've dropped the idea of > testing for less(1) in non-portable mandoc because we know less(1) is > in base.[1] Configuration testing is never needed in a base system. It may sometimes linger when importing third-party software into base, but even then, it is often deleted. > I'm not certain I understand why they're passing /dev/null as > a tagsfile. Maybe to test if any -T throws any error? Exactly, that's what i wrote the HAVE_LESS_T test in mandoc-portable for: to test whether less(1) has (support for) -T. > (I also don't actually understand the redirection to file > descriptor 3. Cluebats welcome, on- or off-list.) $ sed -n /Output/,/3:/p configure # Output file descriptor usage: # 1 (stdout): config.h, Makefile.local # 2 (stderr): original stderr, usually to the console # 3: config.log Anyway, i committed the patch to OpenBSD, thanks to all who provided feedback. No need to discuss mandoc-portable on tech@ any further. Yours, Ingo
nsd 4.3.2
Tests, OKs?
diff --git doc/ChangeLog doc/ChangeLog
index 09ea79bafd3..ba80174afdf 100644
--- doc/ChangeLog
+++ doc/ChangeLog
@@ -1,3 +1,69 @@
+7 July 2020: Wouter
+ - Tag for 4.3.2rc1.
+
+6 July 2020: Wouter
+ - Fix compile includes for xfr-inspect tool on FreeBSD.
+ - Add tpkg/run_vm.sh that runs test when in a virtual machine.
+ - Merge #112 from jaredmauch: log old and new serials when NSD
+ rejects an IXFR due to an old serial number.
+ - Fix bug034 test for vm test changes.
+
+22 June 2020: Wouter
+ - Remove errno reset behaviour from sendmmsg and recvmmsg
+ replacement functions.
+ - Fix unit test for different nsd-control-setup -h exit code.
+
+19 June 2020: Wouter
+ - Merge #108 from Nomis: Make the max-retry-time description clearer.
+ - Retry when udp send buffer is full to wait until buffer space is
+ available.
+
+18 June 2020: Wouter
+ - Do not log EAGAIN errors for sendmmsg, to stop log spam on OpenBSD.
+
+17 June 2020: Wouter
+ - Fix #107: nsd -v shows configure line, openssl version and libevent
version.
+
+27 May 2020: Wouter
+ - Fix unlink of pidfile warning if not possible due to permissions,
+ nsd can display the message at high verbosity levels.
+ - Update contrib/nsd.service for chown of nsd.log and /var/log in
+ ReadWritePaths.
+ - Removed contrib/nsd.service, example is too complicated and not
+ useful.
+
+15 May 2020: Wouter
+ - Merge PR#102 from and0x000: add missing default in documentation
+ for drop-updates.
+ - Fix checkconf test for log-only-syslog option.
+
+14 May 2020: Wouter
+ - Document default value for tcp-timeout.
+
+13 May 2020: Jeroen
+ - Fix #99: Fix copying of socket properties with reuseport enabled.
+
+24 April 2020: Wouter
+ - Fix #97: EDNS unknown version: query not in response.
+
+21 April 2020: Wouter
+ - Fix #96: log-only-syslog: yes sets to only use syslog, fixes
+ that the default configuration and systemd results in duplicate
+ log messages.
+
+20 April 2020: Wouter
+ - Fix #95: Removed make test check because tpkg not included in
+ release tarballs.
+ - Fix unused parameter compile warnings.
+
+16 April 2020: Wouter
+ - Tag for 4.3.1 release and track 4.3.2 release in code repository.
+ - note sha256 digest algo use in makedist.sh.
+ - Fix for posix shell syntax for trap in nsd-control-setup.
+ - Fix to omit the listen-on lines from log at startup, unless verbose.
+ - Fix uninitialised values for bindtodevice option at startup with
+ reuseport and multiple interfaces.
+
8 April 2020: Wouter
- Tag for 4.3.1rc2.
diff --git Makefile.in Makefile.in
index 4e6915d4461..39076034e6c 100644
--- Makefile.in
+++ Makefile.in
@@ -81,7 +81,7 @@ MANUALS=nsd.8 nsd-checkconf.8 nsd-checkzone.8 nsd-control.8
nsd.conf.5
COMMON_OBJ=answer.o axfr.o buffer.o configlexer.o configparser.o dname.o dns.o
edns.o iterated_hash.o lookup3.o namedb.o nsec3.o options.o packet.o query.o
rbtree.o radtree.o rdata.o region-allocator.o rrl.o tsig.o tsig-openssl.o udb.o
udbradtree.o udbzone.o util.o bitset.o popen3.o
XFRD_OBJ=xfrd-disk.o xfrd-notify.o xfrd-tcp.o xfrd.o remote.o $(DNSTAP_OBJ)
NSD_OBJ=$(COMMON_OBJ) $(XFRD_OBJ) difffile.o ipc.o mini_event.o netio.o nsd.o
server.o dbaccess.o dbcreate.o zlexer.o zonec.o zparser.o
-ALL_OBJ=$(NSD_OBJ) nsd-checkconf.o nsd-checkzone.o nsd-control.o nsd-mem.o
+ALL_OBJ=$(NSD_OBJ) nsd-checkconf.o nsd-checkzone.o nsd-control.o nsd-mem.o
xfr-inspect.o
NSD_CHECKCONF_OBJ=$(COMMON_OBJ) nsd-checkconf.o
NSD_CHECKZONE_OBJ=$(COMMON_OBJ) $(XFRD_OBJ) dbaccess.o dbcreate.o difffile.o
ipc.o mini_event.o netio.o server.o zonec.o zparser.o zlexer.o nsd-checkzone.o
NSD_CONTROL_OBJ=$(COMMON_OBJ) nsd-control.o
@@ -193,9 +193,6 @@ audit: nsd nsd-checkconf nsd-checkzone nsd-control nsd-mem
checksec
./checksec --file=nsd-control
./checksec --file=nsd-mem
-test check: cutest
- ./cutest
-
clean:
rm -f *.o $(TARGETS) $(MANUALS) cutest popen3_echo udb-inspect
xfr-inspect nsd-mem
diff --git config.h.in config.h.in
index 67cd876e427..35cd47f2e82 100644
--- config.h.in
+++ config.h.in
@@ -9,6 +9,9 @@
/* NSD default chroot directory */
#undef CHROOTDIR
+/* Command line arguments used with configure */
+#undef CONFCMDLINE
+
/* NSD config dir */
#undef CONFIGDIR
diff --git configlexer.lex configlexer.lex
index 8469bbce35d..bb4dd3499c4 100644
--- configlexer.lex
+++ configlexer.lex
@@ -215,6 +215,7 @@ identity{COLON} { LEXOUT(("v(%s) ", yytext));
return VAR_IDENTITY;}
version{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_VERSION;}
nsid{COLON}{ LEXOUT(("v(%s) ", yytext)); return VAR_NSID;}
logfile{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_LOGFILE;}
+log-only-syslog{COLON} { LEXOUT(("v(%s) ", yytext)); return
Re: pfctl.8: mention hostid and checksum for -s info
Hello, On Mon, Jul 20, 2020 at 03:23:41PM +0200, Klemens Nanni wrote: > Getting the checksum with pfctl(8) is either in your finger's muscle > memory or takes guess work as the manual doesn't mention it. > > I grepped the code to see that I need `-s info' with `-v'. > > (Setting) hostid is described in pf.conf(5) but pfctl(8) doesn't tell us > how to print it, there's merely an example for killing states based on > it. > > Complete the description of `-s info' to mention and describe both such > that grepping for it in the manual pager yields any results. > > Feedback? OK? > fine with me. OK sashan
pfctl.8: mention hostid and checksum for -s info
Getting the checksum with pfctl(8) is either in your finger's muscle memory or takes guess work as the manual doesn't mention it. I grepped the code to see that I need `-s info' with `-v'. (Setting) hostid is described in pf.conf(5) but pfctl(8) doesn't tell us how to print it, there's merely an example for killing states based on it. Complete the description of `-s info' to mention and describe both such that grepping for it in the manual pager yields any results. Feedback? OK? Index: pfctl.8 === RCS file: /cvs/src/sbin/pfctl/pfctl.8,v retrieving revision 1.180 diff -u -p -r1.180 pfctl.8 --- pfctl.8 15 Jan 2020 14:39:41 - 1.180 +++ pfctl.8 20 Jul 2020 13:17:15 - @@ -403,7 +403,10 @@ Show the contents of the source tracking Show filter information (statistics and counters). When used together with .Fl v , -source tracking statistics are also shown. +source tracking statistics as well as the firewall's 32-bit hostid +number and the main ruleset's MD5 checksum for use with +.Xr pfsync 4 +are also shown. .It Fl s Cm labels Show per-rule statistics (label, evaluations, packets total, bytes total, packets in, bytes in, packets out, bytes out, state creations) of
Re: wsfontload(8): display number of characters in a loaded font
On Fri, Jul 17, 2020 at 11:04:07AM +0100, Stuart Henderson wrote:
> Seems useful. While it's not especially likely anyone is parsing the output
> of this, just in case they are it's usually more admin-friendly to add a new
> column at the end unless there's a good reason not to.
Good point, here is a revised diff adding the new column at the end,
and the new output as follow:
# Name Encoding W HChars
0 Boldface ibm 8 16 254
1 Spleen 6x12 iso 6 12 96
2 Spleen 8x16 iso 8 16 224
3 Spleen 12x24 iso 12 24 224
4 Spleen 16x32 iso 16 32 224
5 Spleen 32x64 iso 32 64 224
Comments? OK?
Index: usr.sbin/wsfontload/wsfontload.c
===
RCS file: /cvs/src/usr.sbin/wsfontload/wsfontload.c,v
retrieving revision 1.21
diff -u -p -r1.21 wsfontload.c
--- usr.sbin/wsfontload/wsfontload.c28 Jun 2019 13:32:51 - 1.21
+++ usr.sbin/wsfontload/wsfontload.c20 Jul 2020 11:41:32 -
@@ -141,7 +141,8 @@ main(int argc, char *argv[])
if (list) {
i = 0;
- p = " # Name Encoding W H";
+ p = " # Name Encoding" \
+ " W HChars";
do {
f.index = i;
res = ioctl(wsfd, WSDISPLAYIO_LSFONT, );
@@ -151,10 +152,11 @@ main(int argc, char *argv[])
puts(p);
p = NULL;
}
- printf("%2d %-32s %8s %2d %2d\n",
+ printf("%2d %-32s %8s %2d %2d %8d\n",
f.index, f.name,
encodings[f.encoding].name,
- f.fontwidth, f.fontheight);
+ f.fontwidth, f.fontheight,
+ f.numchars);
}
}
i++;
Re: pf: remove ptr_array from struct pf_ruleset
Hello Klemens.
I took a closer look at your change and related area. Below is an alternate
way to fix the bug you've found.
there are few details worth to note:
ptr_array matters to main ruleset only, because it is the only
ruleset covered by MD5 sum for pfsync.
it looks like we should also recompute the MD5sum if we remove the rule
from the main ruleset
my diff introduces a new member of pr_ruleset structure, which keeps the
array size so we can pass it to free(9f) later.
I have no pfsync deployment readily available for testing. Will be glad
if you can give my diff a try.
thanks a lot
regards
sashan
8<--8<--8<-8<---
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index a486745bc1c..00a4f70e7db 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -332,8 +332,32 @@ pf_purge_rule(struct pf_rule *rule)
pf_rm_rule(ruleset->rules.active.ptr, rule);
ruleset->rules.active.rcount--;
- TAILQ_FOREACH(rule, ruleset->rules.active.ptr, entries)
- rule->nr = nr++;
+
+ /*
+* pf_chksum calculation is copied and modified from
+* pf_setup_pfsync_matching(), which is part of ruleset commit
+* operation.
+*/
+ if (ruleset == _main_ruleset) {
+ MD5_CTX ctx;
+ u_int8_tdigest[PF_MD5_DIGEST_LENGTH];
+
+ MD5Init();
+
+ TAILQ_FOREACH(rule, ruleset->rules.active.ptr, entries) {
+ rule->nr = nr++;
+ pf_hash_rule(, rule);
+ ruleset->rules.active.ptr_array[rule->nr] = rule;
+ }
+
+ MD5Final(digest, );
+ memcpy(pf_status.pf_chksum, digest,
sizeof(pf_status.pf_chksum));
+ } else {
+ TAILQ_FOREACH(rule, ruleset->rules.active.ptr, entries) {
+ rule->nr = nr++;
+ }
+ }
+
ruleset->rules.active.ticket++;
pf_calc_skip_steps(ruleset->rules.active.ptr);
pf_remove_if_empty_ruleset(ruleset);
@@ -804,6 +828,7 @@ pf_commit_rules(u_int32_t ticket, char *anchor)
{
struct pf_ruleset *rs;
struct pf_rule *rule, **old_array;
+ size_t old_array_sz;
struct pf_rulequeue *old_rules;
int error;
u_int32_told_rcount;
@@ -827,13 +852,16 @@ pf_commit_rules(u_int32_t ticket, char *anchor)
old_rules = rs->rules.active.ptr;
old_rcount = rs->rules.active.rcount;
old_array = rs->rules.active.ptr_array;
+ old_array_sz = rs->rules.active.array_sz;
rs->rules.active.ptr = rs->rules.inactive.ptr;
rs->rules.active.ptr_array = rs->rules.inactive.ptr_array;
rs->rules.active.rcount = rs->rules.inactive.rcount;
+ rs->rules.active.array_sz = rs->rules.inactive.array_sz;
rs->rules.inactive.ptr = old_rules;
rs->rules.inactive.ptr_array = old_array;
rs->rules.inactive.rcount = old_rcount;
+ rs->rules.inactive.array_sz = old_array_sz;
rs->rules.active.ticket = rs->rules.inactive.ticket;
pf_calc_skip_steps(rs->rules.active.ptr);
@@ -843,10 +871,12 @@ pf_commit_rules(u_int32_t ticket, char *anchor)
while ((rule = TAILQ_FIRST(old_rules)) != NULL)
pf_rm_rule(old_rules, rule);
if (rs->rules.inactive.ptr_array)
- free(rs->rules.inactive.ptr_array, M_TEMP, 0);
+ free(rs->rules.inactive.ptr_array, M_TEMP,
+ rs->rules.inactive.array_sz);
rs->rules.inactive.ptr_array = NULL;
rs->rules.inactive.rcount = 0;
rs->rules.inactive.open = 0;
+ rs->rules.inactive.array_sz = 0;
pf_remove_if_empty_ruleset(rs);
/* queue defs only in the main ruleset */
@@ -864,10 +894,14 @@ pf_setup_pfsync_matching(struct pf_ruleset *rs)
MD5Init();
if (rs->rules.inactive.ptr_array)
- free(rs->rules.inactive.ptr_array, M_TEMP, 0);
+ free(rs->rules.inactive.ptr_array, M_TEMP,
+ rs->rules.inactive.array_sz);
rs->rules.inactive.ptr_array = NULL;
+ rs->rules.inactive.array_sz = 0;
if (rs->rules.inactive.rcount) {
+ rs->rules.inactive.array_sz =
+ rs->rules.inactive.rcount * sizeof(caddr_t);
rs->rules.inactive.ptr_array =
mallocarray(rs->rules.inactive.rcount, sizeof(caddr_t),
M_TEMP, M_NOWAIT);
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index f06a1fa0e07..acdafcbc971 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -926,6 +926,7 @@ struct pf_ruleset {
struct pf_rule **ptr_array;
u_int32_trcount;
u_int32_tticket;
+ size_t
Re: iwn: fix off-by-one in antenna calibration for iwn5000
On Fri, Jul 17, 2020 at 12:50:04PM +0200, Holger Mikolon wrote: > I came across this by reading the code if_iwn.c and DPRINTFs on > a kernel with IWN_DEBUG. > > IWN_LSB() returns an index starting with 1, however the arrays used > later on (noise and gain in iwn5000_set_gains()) start with 0. The > current code accounts for this difference when setting the antenna > gain by accessing cmd.gain[i - 1]. However the noise array is accessed > with noise[i], the chainmask is as well checked against i and more > importantly the overall for() loop iterates wrongly over the antennas by > always starting with i=2 (the third antenna). One consequence is, that > gain calibration never happens in case of only two antennas. > > Secondly, the final DPRINTF in iwn5000_set_gains() assumes a two-antenna > setup. In my case three antennas are connected. I don't know if there > are iwn setups with one antenna, but the DPRINTF wouldn't make sense > there at all. Hence I propose to move this DPRINTF up where it makes > more sense (and adjust it to the new place). > > My diff below fixes the said off-by-one and DPRINTF. Additionally > it adds another DPRINTF which I felt useful while debugging and > it extends a comment - those additions may be skipped of course. > > Here is few details of my laptop (cvs updated and kernel built today): > > $ dmesg | grep iwn0 > iwn0 at pci2 dev 0 function 0 "Intel WiFi Link 5300" rev 0x00: msi, MIMO > 3T3R, MoW, address 00:21:6a:56:2b:36 > > $ sysctl hw | grep -e machine -e model -e vendor -e product > hw.machine=amd64 > hw.model=Intel(R) Core(TM)2 Duo CPU P8600 @ 2.40GHz > hw.vendor=Dell Inc. > hw.product=Studio 1555 > > Let me know if you need a full dmesg or anything else. > Thanks! This is a pretty obvious fix. I have committed it and afterwards tweaked the multi-line comment you've added to conform to style(9). Something to keep in mind for future patches ;-) Regarding DPRINTFs: I try to avoid committing them into the tree because they add a lot of noise as they accumulate. And they are rarely useful outside of the context of the debugging session they were written for. Note that iwn(4) in particular contains so many DPRINTFs that just turning on IWN_DEBUG may cause the driver to miss the WPA handshake timeout while the console is being displayed (in particular if 'ifconfig iwn0 debug' is enabled, too). We have already reached the point where the many DPRINTFs prevent proper operation of this driver. I kept your DPRINTFs for now. But it would make sense to do a sweep over the entire iwn(4) driver and eliminate some or most of them. I did that for iwm(4) many years ago and never had any regrets.
