Re: diff: cleanup type handling

2020-12-12 Thread Alexander Bluhm
On Sat, Dec 12, 2020 at 02:25:03PM +0100, Jan Klemkow wrote: > The type of the local variable hash in pf_map_addr() has right length > but the wrong type. This diff uses the correct type and removes the > useless casts. Both functions uses hash as pf_addr, so no cast is > needed. > > OK? OK blu

Re: diff: replace useless use of MCLGETL with MCLGET

2020-12-14 Thread Alexander Bluhm
On Sat, Dec 12, 2020 at 02:05:48PM +0100, Jan Klemkow wrote: > Thus, this diff removes '(void)' from the MCLGET macro > -#define MCLGET(m, how) (void) m_clget((m), (how), MCLBYTES) > +#define MCLGET(m, how) m_clget((m), (how), MCLBYTES) The MCLGET API is to add a cluster to an existing mbuf. Retu

Re: Kernel panic with i386 on latest snapshot

2020-12-15 Thread Alexander Bluhm
On Tue, Dec 15, 2020 at 06:57:03PM +0100, Mark Kettenis wrote: > Does the diff below fix this? I can reproduce the panic and your diff fixes it. Usually my regress machines do not trigger it as I do not install firmware. fw_update and reboot makes it crash. bluhm OpenBSD 6.8-current (GENERIC.M

amd64 pamp panic messages

2020-12-16 Thread Alexander Bluhm
Hi, during all my pmap crashes, I sometimes get this strange address. panic: pmap_remove_pte: unmanaged page marked PG_PVLIST, va = 0x5d155753000, pa = 0xfdfdfdfdfd000 I think we should not clear bits in a panic messages. Debugging with the full picture is easier. While there make the panics

regress print target name

2020-12-16 Thread Alexander Bluhm
Hi, When debugging tests, it is useful to see the target name and which output belongs to it. A lot of my tests have echo lines, but I think this is better done in the framework. Then all tests behave simmilar. I would remove the echos from the Makefiles afterwards. ok? bluhm Index: share/mk

Re: regress print target name

2020-12-17 Thread Alexander Bluhm
On Wed, Dec 16, 2020 at 04:42:59PM +0100, Alexander Bluhm wrote: > When debugging tests, it is useful to see the target name and which > output belongs to it. A small addition: Run setup_once targets in a sepearate block with headline before all other targets. ok? bluhm Index: sh

amd64 pmap pv_entry SLIST

2020-12-17 Thread Alexander Bluhm
Hi, Can we convert the pv_entry list in amd64 pmap into an SLIST? I think the code with macros is easier to read. ok? bluhm Index: arch/amd64//amd64/pmap.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/pmap.c,v re

Re: converting uvm_km_valloc to km_alloc

2020-12-18 Thread Alexander Bluhm
On Fri, Dec 18, 2020 at 10:36:28AM +1000, Jonathan Matthew wrote: > Here are a couple of relatively easy ones, applying changes from r1.86 of > amd64's acpi_machdep.c to i386 and arm64. I've tested i386 but it turns out I > don't have any arm64 machines with acpi. A machine like this? Something

IPsec PMTU and reject route

2020-12-19 Thread Alexander Bluhm
Hi, In revision 1.87 of ip_icmp.c claudio@ added ignoring reject routes to icmp_mtudisc_clone(). Otherwise TCP would clone these routes for PMTU discovery. They will not work, even after dynamic routing has found a better route than the reject route. With IPsec the use case is different. First

IPv6 pf_test EACCES

2020-12-21 Thread Alexander Bluhm
Hi, A while ago we decided to pass EACCES to uerland if pf blocks a packet. IPv6 still has the old EHOSTUNREACH code. Use the same errno for dropped IPv6 packets as in IPv4. ok? bluhm Index: netinet6/ip6_output.c === RCS file: /d

Re: netstat - proto ip record

2020-12-23 Thread Alexander Bluhm
On Wed, Dec 16, 2020 at 05:24:50PM +0100, Claudio Jeker wrote: > On Wed, Dec 16, 2020 at 03:54:04PM +, Stuart Henderson wrote: > > On 2020/12/16 16:43, Salvatore Cuzzilla wrote: > > > Hi folks, > > > > > > is there any process associated with this netstat record? > > > btw, what's the meaning

IPsec IPv6 PMTU

2020-12-24 Thread Alexander Bluhm
Hi, This diff makes path MTU discovery work for IPv6 IPsec ESP over IPv4 tunnel. Basically it ports code from v4 to v6. It also makes v4 and v6 code look simmilar. If you want, I can split this for easier review. ok? bluhm Index: netinet/icmp6.h ==

Re: IPsec IPv6 PMTU

2020-12-27 Thread Alexander Bluhm
On Thu, Dec 24, 2020 at 10:54:59PM +0100, Alexander Bluhm wrote: > It also makes v4 and v6 code look simmilar. If you want, I can > split this for easier review. This is the part of the diff that creates a path MTU host route for IPv6. Basically the code is copied from IPv4 and adapted.

Re: Thread local data setup and destruct

2020-12-31 Thread Alexander Bluhm
On Tue, Dec 29, 2020 at 04:07:19PM +0100, Otto Moerbeek wrote: > This workds better, checking the flags does not work if the thread is > already on the road to desctruction. This diff survived a full regress run on amd64. bluhm > Index: asr/asr.c > ===

Re: uvm_fault: amap & anon locking

2021-01-01 Thread Alexander Bluhm
On Wed, Dec 30, 2020 at 11:19:41AM -0300, Martin Pieuchot wrote: > This has been extensively tested as part of the unlocking diff I sent to > many developers. However, I'd appreciate if you could test again because > this diff doesn't include WITNESS and do not unlock the fault handler. Passed re

Re: pf route-to issues

2021-01-03 Thread Alexander Bluhm
On Sun, Jan 03, 2021 at 02:00:00PM +1000, David Gwynne wrote: > On Tue, Oct 20, 2020 at 09:27:09AM +1000, David Gwynne wrote: > We've been running this diff in production for the last couple of > months, and it's been solid for us so far. Ignoring the fixes for > crashes, I personally find it a lot

Re: pf route-to issues

2021-01-03 Thread Alexander Bluhm
On Sun, Jan 03, 2021 at 06:56:20PM +0100, Alexander Bluhm wrote: > I am currently running a full regress to find more fallout. These regress tests fail: sys/net/pf_forward sys/net/pf_fragment sbin/pfctl The first two are easy to fix. That means my tests using route-to work fine with your d

Re: convert i386 fix_f00f() uvm_km_zalloc

2021-01-03 Thread Alexander Bluhm
On Mon, Jan 04, 2021 at 10:00:25AM +1000, Jonathan Matthew wrote: > I don't have a real 586, but I can tell qemu to pretend to be one, > which at least executes this code. You can run regress/sys/arch/i386/f00f/ . > Using kd_waitok here seems suspect, because if we're out of memory > this early I

Re: pf route-to issues

2021-01-04 Thread Alexander Bluhm
On Mon, Jan 04, 2021 at 11:46:16AM +0100, Alexandr Nedvedicky wrote: > > let's put this in and then i'll have a look. ok by me. > bluhm's diff is fine with me. Refactoring is commited, here is the remaining kernel diff after merge. bluhm Index: net/if_pfsync.c ===

Re: pf route-to issues

2021-01-04 Thread Alexander Bluhm
On Mon, Jan 04, 2021 at 03:26:15PM +0100, Alexandr Nedvedicky wrote: > you refactoring diff requires a minor finishing touch to keep the > stuff compiling: Did I commit something that does not compile? I just made cvs update on another machine. There it worked. The rt_kif in pf_state still exis

Re: pf route-to issues

2021-01-04 Thread Alexander Bluhm
On Mon, Jan 04, 2021 at 04:32:45PM +0100, Alexandr Nedvedicky wrote: > so either rt_kif must stay for a while, or your new diff (rebased on top > of > stuff committed already) must be expanded by the nit pick I've sent. The diff I sent contains this bit. I still think the merge bug is on

Re: pf route-to issues

2021-01-04 Thread Alexander Bluhm
On Mon, Jan 04, 2021 at 11:21:50PM +1000, David Gwynne wrote: > this chunk pops out as a standalone change. > > having pf_find_state() return PF_PASS here means the callers short > circuit and let the packet go through without running it through the > a lot of the state handling, which includes th

Re: pf route-to issues

2021-01-08 Thread Alexander Bluhm
On Tue, Jan 05, 2021 at 10:05:39PM +1000, David Gwynne wrote: > If the idea is to avoid running most of pf_test again if route-to is > applied during ip_output, I think this tweaked diff is simpler. Is there > a valid use case for running some of pf_test again after route-to is > applied? I found

pf log user and group

2021-01-11 Thread Alexander Bluhm
Hi, Sometimes an uid is logged in pflog(4) although the logopt of the rule does not specify it. Check the option again for the log rule in case another rule has triggered a socket lookup. Remove logopt group, it is not documented and cannot work as struct pfloghdr does not contain a gid. Rename

pf af-to sysctl forwarding

2021-01-15 Thread Alexander Bluhm
Hi, sysctl net.inet.ip.forwarding is checked before ip_input() passes the packet to ip_forward(). But with an af-to rule, pf(4) calls ip_forward() directly. I think we should check the sysctl also in pf to get consistent behaviour. ok? bluhm Index: net/pf.c ===

sysctl ip.forwarding 2

2021-01-15 Thread Alexander Bluhm
Hi, As documented in sysctl(2) net.inet.ip.forwarding can be 2. netinet/ip_output.c:448 if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) && Current input validation prevents this. # sysctl net.inet.ip.forwarding=2 sysctl: net.inet.ip.forwarding: Invalid argument Also

Re: pf af-to sysctl forwarding

2021-01-15 Thread Alexander Bluhm
On Fri, Jan 15, 2021 at 03:24:43PM +0100, Klemens Nanni wrote: > Existing routers doing NAT64 for IPv6-only networks will require > `net.inet.ip.forwarding=1' for NAT64 to work. Actually you will need both of them. When sending "IPv6 -> pf-router -> IPv4" you need ip forwarding as pf translates t

Re: tell pfctl(8) route-to and reply-to accept next-hop only

2021-01-15 Thread Alexander Bluhm
On Tue, Jan 12, 2021 at 08:45:22PM +0100, Alexandr Nedvedicky wrote: > I think bluhm@ and dlg@ have committed part of that change already. I have only commited a refactoring change. Next step in kernel would be to remove the check in pf_find_state() and see what happens. I was waiting for dlg@ t

Re: Add if_mreqn support to IP_MULTICAST_IF

2021-01-15 Thread Alexander Bluhm
On Fri, Jan 15, 2021 at 03:02:37PM +0100, Claudio Jeker wrote: > On Fri, Jan 15, 2021 at 02:53:17PM +0100, Claudio Jeker wrote: > > I forgot to add ip_mreqn support to IP_MULTICAST_IF and so the > > IP_ADD_MEMBERSHIP change is not fixing all the issues I have. > > > > Linux supports calling IP_MUL

pflog remove translation

2021-01-18 Thread Alexander Bluhm
Hi, pflog(4) tries to log the translated packet with rdr-to, nat-to, and af-to applied. Therefore it creates a mbuf chain on the stack with a partial copy. This might have been a good idea for plain IPv4 10 years ago. But now the concept fails miserably due to: - IP options - extension header

tcpdump pflog af and rewritten addresses

2021-01-18 Thread Alexander Bluhm
Hi, tcpdump pflog with addresses rewritten by rdr-to, nat-to, or af-to is broken. 1. Fix address family of the packet in af-to rules: before: 19:26:37.620926 169.254.0.14 > 169.254.0.14: icmp: echo request 19:26:37.620946 bad-ip6-version 4 19:26:37.620963 fc00::23 > fc00::24: icmp6: echo request

broadcast simplex checksum

2021-01-19 Thread Alexander Bluhm
Hi, Simplex interfaces reinject broadcast packets back into the IP stack. As this is a software features, no hardware checksumming occurs. So local broadcast packets are dropped with wrong checksum if the underlying hardware supports checksumming. Do software checksumming in ip_output() if the

IPv6 IPsec path MTU discovery

2021-01-20 Thread Alexander Bluhm
Hi, This part of the IPv6 IPsec path MTU discovery is for the case where the router is between the tunnel endpoints. Basically it handles ICMP6 packets for ESP. Originally this diff came from markus@. ok? bluhm Index: netinet/ip_ipsp.h =

Re: [External] : Re: pf route-to issues

2021-01-25 Thread Alexander Bluhm
Hi, Some personal thoughts. I am happy when pf route-to gets simpler. Especially I have never understood what this address@interface syntax is used for. I cannot estimate what configuration is used by our cutomers in many installations. Simple syntax change address@interface -> address of next

Re: pf route-to issues

2021-01-25 Thread Alexander Bluhm
On Fri, Jan 22, 2021 at 06:07:59PM +1000, David Gwynne wrote: > --- sys/conf/GENERIC 30 Sep 2020 14:51:17 - 1.273 > +++ sys/conf/GENERIC 22 Jan 2021 07:33:30 - > @@ -82,6 +82,7 @@ pseudo-device msts1 # MSTS line discipl > pseudo-deviceendrun 1 # EndRun

Re: pf route-to issues

2021-01-26 Thread Alexander Bluhm
On Tue, Jan 26, 2021 at 10:39:30AM +1000, David Gwynne wrote: > > But what about dup-to? The packet is duplicated for both directions. > > I guess the main use case for dup-to is implementing a monitor port. > > There you have to pass packets stateless, otherwise it would not > > work anyway. The

Re: systat(1): improve parsing of delay value

2021-01-26 Thread Alexander Bluhm
On Mon, Jan 25, 2021 at 11:17:04AM +0100, Martijn van Duren wrote: > if (argc == 1) { > - double del = atof(argv[0]); > - if (del == 0) > + delay = strtodnum(argv[0], 0, UINT32_MAX / 100, &errstr); > + if (errstr != NULL) >

Re: tiny pf_route{,6} tweak

2021-01-26 Thread Alexander Bluhm
On Wed, Jan 27, 2021 at 11:14:51AM +1000, David Gwynne wrote: > On Wed, Jan 27, 2021 at 11:13:12AM +1000, David Gwynne wrote: > > when pf_route (and pf_route6) are supposed to handle forwarding the > > packet (ie, for route-to or reply-to rules), they take the mbuf > > away from the calling code pa

Re: don't run dup-to generated packets through pf_test in pf_route{,6}

2021-01-26 Thread Alexander Bluhm
On Wed, Jan 27, 2021 at 11:31:27AM +1000, David Gwynne wrote: > this was discussed as part of the big route-to issues thread. i think > it's easy to break out and handle separately now. > > the diff does what the subject line says. it seems to work as expected > for me. i don't see weird state iss

Re: if pf_route{,6} route isn't valid, generate an icmp error

2021-01-27 Thread Alexander Bluhm
On Wed, Jan 27, 2021 at 04:41:01PM +1000, David Gwynne wrote: > at the moment if the route is invalid, we drop the packet. this > generates an icmp error. > > ok? OK bluhm@ > Index: pf.c > === > RCS file: /cvs/src/sys/net/pf.c,v > r

Re: have pf_route bail out if it resolves a route with RTF_LOCAL set

2021-01-28 Thread Alexander Bluhm
On Thu, Jan 28, 2021 at 09:57:33AM +1000, David Gwynne wrote: > calling if_output with a route to a local IP is confusing, and I'm not > sure it makes sense anyway. > > this treats a an RTF_LOCAL route like an invalid round and drops the > packet. > > ok? Are you sure that it does not break any

Re: pf: route-to IPs, not interfaces

2021-01-28 Thread Alexander Bluhm
On Thu, Jan 28, 2021 at 10:54:30PM +1000, David Gwynne wrote: > this is the diff from the "pf route-to issues" thread, but on it's own. I think we should make progress and commit something. > the caveat is that route-to becomes tied to pass rules that create > state, like rdr-to and nat-to.

Re: systat(1): improve parsing of delay value

2021-01-28 Thread Alexander Bluhm
On Thu, Jan 28, 2021 at 09:06:51PM +0100, Martijn van Duren wrote: > Thanks for checking. Should be fixed below. OK bluhm@ > Index: main.c > === > RCS file: /cvs/src/usr.bin/systat/main.c,v > retrieving revision 1.72 > diff -u -p -r1

Re: have pf_route bail out if it resolves a route with RTF_LOCAL set

2021-01-29 Thread Alexander Bluhm
On Fri, Jan 29, 2021 at 10:53:09AM +1000, David Gwynne wrote: > > Are you sure that it does not break any use case? I have seen so > > much strange stuff. What is the advantage? > > The current behaviour is lucky at best, and quirky at worst. Usually I > would agree with you that breaking stuff

IPsec IPv6 Path MTU discovery

2021-01-29 Thread Alexander Bluhm
Hi, This fixes path MTU discovery for ESP tunneled in IPv6. In IPv6 we always want short TCP segments or fragments encapsulated in ESP instead off fragmented ESP packets. ok? bluhm Index: netinet/ip_output.c === RCS file: /data/mi

Re: broadcast simplex checksum

2021-01-31 Thread Alexander Bluhm
On Mon, Feb 01, 2021 at 08:08:56AM +1300, Richard Procter wrote: > - Might the rule disabling checksum offload for broadcasts on IFF_SIMPLEX > interfaces be weakened to disable checksum offload for all broadcast > packets instead? I just copied the condition from ether_resolve():

Re: Remove obsolete vnode opv declarations

2021-02-01 Thread Alexander Bluhm
On Mon, Feb 01, 2021 at 02:14:24PM +, Visa Hankala wrote: > This removes obsolete vnode operation vector declarations from > header . The functions were removed in r1.28 of vfs_init.c. > > OK? OK bluhm@ > Index: sys/systm.h > ==

tcpbench -D

2021-02-04 Thread Alexander Bluhm
Hi, I would like to analyse tcpbench(1) TCP connections. So I copied the nc -D socket debug option. ok? bluhm Index: usr.bin/tcpbench/tcpbench.1 === RCS file: /data/mirror/openbsd/cvs/src/usr.bin/tcpbench/tcpbench.1,v retrieving r

reference trpt(8) in getsockopt(2)

2021-02-04 Thread Alexander Bluhm
Hi, I always forget the name of trpt(8). It should be refereced in the SO_DEBUG section of getsockopt(2). ok? bluhm Index: lib/libc/sys/getsockopt.2 === RCS file: /data/mirror/openbsd/cvs/src/lib/libc/sys/getsockopt.2,v retrieving

Re: reference trpt(8) in getsockopt(2)

2021-02-04 Thread Alexander Bluhm
On Thu, Feb 04, 2021 at 12:34:22PM +0100, Claudio Jeker wrote: > Also should we export the tcp_debug buffer via sysctl so that > trpt can run without kern.allowkmem? I have set kern.allowkmem on my development and testing machines. But of course a sysctl that always works would make this often for

Re: broadcast simplex checksum

2021-02-05 Thread Alexander Bluhm
On Mon, Feb 01, 2021 at 02:04:51AM +0100, Alexander Bluhm wrote: > On Mon, Feb 01, 2021 at 08:08:56AM +1300, Richard Procter wrote: > > - Might the rule disabling checksum offload for broadcasts on IFF_SIMPLEX > > interfaces be weakened to disable checksum offload for all broadcas

ifg_refcnt atomic operation

2021-02-05 Thread Alexander Bluhm
Hi, When I replace the ++ and -- of ifg_refcnt with an atomic operation, it fixes this syzkaller panic. https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43 Without the fix "syz-execprog -repeat=0 -procs=8 repro-pfi.syz" crashes my vmm in a few seconds. With the diff I

Re: ifg_refcnt atomic operation

2021-02-06 Thread Alexander Bluhm
On Sat, Feb 06, 2021 at 05:04:20PM +1000, David Gwynne wrote: > refcnt_init starts counting at 1, while the existing code starts at 0. Do > the crashes stop because we never fully release all the references and > never free it now? You are absolutely right. I was too optimistic. Correct diff is

Re: broadcast simplex checksum

2021-02-06 Thread Alexander Bluhm
On Sat, Feb 06, 2021 at 08:26:35PM +1300, richard.n.proc...@gmail.com wrote: > I'm ok with your latest diff as-is. I prefer a slightly different > direction, see below, but not enough to object. I have commited my diff as is. It is better if you expess your arguments yourself in the comment whe

Re: ifg_refcnt atomic operation

2021-02-06 Thread Alexander Bluhm
On Sat, Feb 06, 2021 at 05:58:35PM +0300, Vitaliy Makkoveev wrote: > I???m not sure it should be atomic. It seems groups require their own > lock and this lock should be held while we perform if_addgroup() and > if_delgroup(). I also think that atomic refcounting is not needed here. But it does n

Re: ifg_refcnt atomic operation

2021-02-06 Thread Alexander Bluhm
On Sat, Feb 06, 2021 at 04:44:08PM +0100, Alexander Bluhm wrote: > Or should we go with a self crafted ++ -- refcounting? This would look like this, also fine with me. kasserts are also in refcnt_... API. ok? bluhm Index: net/i

Re: diff: tcp ack improvement

2021-02-08 Thread Alexander Bluhm
On Wed, Feb 03, 2021 at 11:20:04AM +0100, Claudio Jeker wrote: > Just commit it. OK claudio@ > If people see problems we can back it out again. This has huge impact on TCP performance. http://bluhm.genua.de/perform/results/2021-02-07T00%3A01%3A40Z/perform.html For a single TCP connection between

Re: diff: tcp ack improvement

2021-02-08 Thread Alexander Bluhm
On Mon, Feb 08, 2021 at 07:03:59PM +0100, Jan Klemkow wrote: > On Mon, Feb 08, 2021 at 03:42:54PM +0100, Alexander Bluhm wrote: > > On Wed, Feb 03, 2021 at 11:20:04AM +0100, Claudio Jeker wrote: > > > Just commit it. OK claudio@ > > > If people see problems we can back

Re: PF_UNIX sockets unlocking

2021-02-09 Thread Alexander Bluhm
On Thu, Feb 04, 2021 at 03:07:44PM +0300, Vitaliy Makkoveev wrote: > I hope someone else will try it and gives positive feedback which allow > to push it forward. OK bluhm@ > +extern struct rwlock unp_lock; Could you put this declaration into a header file?

Re: PF_UNIX sockets unlocking

2021-02-09 Thread Alexander Bluhm
On Tue, Feb 09, 2021 at 09:14:44PM +0300, Vitaliy Makkoveev wrote: > On Tue, Feb 09, 2021 at 05:20:33PM +0100, Alexander Bluhm wrote: > > > +extern struct rwlock unp_lock; > > > > Could you put this declaration into a header file? > > I see no such sense to do this.

interface group name validation

2021-02-09 Thread Alexander Bluhm
Hi, Next try to fix syzkaller crash https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43 Interface group names must fit into IFNAMSIZ and be unique. But the kernel makes the unique check before trunkating with strlcpy(). So there can be two interfaces groups with the sam

isakmpd link dynamically

2021-02-10 Thread Alexander Bluhm
Hi, Every time we ship a libcrypto erratum, we have to relink isakmpd. I think that isakmpd and iked are in /sbin due to a historic mistake. Probably it is for people who mount /usr via NFS over IPsec. Moving isakmpd to /usr/sbin is hard, linking dynamically is easy. Lines stolen from iked. Is t

Re: isakmpd link dynamically

2021-02-11 Thread Alexander Bluhm
On Wed, Feb 10, 2021 at 04:16:10PM -0700, Theo de Raadt wrote: > When I re-ordered rc in Slovenia many years ago, I got it right. NFS /usr over IPsec cannot work. Without IPsec it is fine. 1. mount -s /usr >/dev/null 2>&1 2. start_daemon syslogd ldattach pflogd nsd unbound ntpd 3. start_daemon is

Re: route sockets: simplify route_attach() error path

2021-02-11 Thread Alexander Bluhm
On Wed, Feb 10, 2021 at 10:51:52PM +0300, Vitaliy Makkoveev wrote: > Do soreserve() before `rop' allocation. It doesn't require protocol > control block be attached to socket. Also we always call `pr_attach' in > thread context so we always have `curproc'. While I found one pr_attach() from TCP i

Re: Possible null deref on pf.c

2021-02-12 Thread Alexander Bluhm
On Fri, Feb 12, 2021 at 01:11:24PM +0100, Claudio Jeker wrote: > On Fri, Feb 12, 2021 at 12:03:49PM +, Ricardo Mestre wrote: > > This was reported on CID 1501718, ifp starts as NULL and then might be > > deref'ed. > This code is strange, the scope for the IPv6 address needs to be pulled > ou

mbuf leak ip_insertoptions

2021-02-22 Thread Alexander Bluhm
Hi, ip_insertoptions() may prepend a mbuf. In this case "goto bad" has to free the new chain. Currently we leak the new mbuf in front of the old chain. NetBSD has fixed this bug here: revision 1.33 date: 1996-10-11 18:19:08 +; author: is; state: Exp; lines:

Re: have m_copydata use a void * instead of caddr_t

2021-02-23 Thread Alexander Bluhm
On Tue, Feb 23, 2021 at 07:31:30PM +1000, David Gwynne wrote: > i'm not a fan of having to cast to caddr_t when we have modern > inventions like void *s we can take advantage of. Shoud you remove all the (caddr_t) casts in the callers then? Without that step this diff does not provide more consist

Re: have m_copydata use a void * instead of caddr_t

2021-02-24 Thread Alexander Bluhm
On Wed, Feb 24, 2021 at 04:27:03PM +1000, David Gwynne wrote: > it's a start though. cocci and i came up with this to push in after. Less casting is better. OK bluhm@ > Index: arch/armv7/sunxi/sxie.c > === > RCS file: /cvs/src/sys/

ip_fragment ip6_fragment

2021-02-26 Thread Alexander Bluhm
Hi, I always bothered me that ip_fragment() and ip6_fragment() behave sligtly differently. Unify them and use an mlist to simplify the fragment list. - The functions ip_fragment() and ip6_fragment() always consume the mbuf. - They free the mbuf and mbuf list in case of an error. - They care abou

sendsyslog kernel buffer

2021-03-06 Thread Alexander Bluhm
Hi Early daemons like dhcpleased, slaacd, unwind, resolvd are started before syslogd. This results in ugly sendsyslog: dropped 1 message logs and the real message is lost. Changing the start order of syslogd and and network daemons is not feasible. A possible solution is a temporary buffer for

Re: sendsyslog kernel buffer

2021-03-08 Thread Alexander Bluhm
I hope to have addressed all issues. On Sun, Mar 07, 2021 at 11:50:24AM +, Visa Hankala wrote: > This copyin() can also result in copying the buffer from userspace twice. > This might not be a problem with log data though. Is double copyin a problem? I think error != EFAULT should catch all

Re: sendsyslog kernel buffer

2021-03-09 Thread Alexander Bluhm
On Mon, Mar 08, 2021 at 11:55:30PM +0300, Vitaliy Makkoveev wrote: > This silently drops message if copyin() fails. Could we count them as > `logstash_dropped??? too? No, as there is no message. Userland made something wrong. At least that is how I understand one of visa@'s remarks. The previou

ntpd adjtime offset race

2021-03-16 Thread Alexander Bluhm
Hi, I am running ntpd as a client with three upstream servers. Some of them are not synchonized and report a time that is off by several seconds. The ntpd client code corrects both T1 and T4 with the current offset returned by adjtime(2) from the kernel. T1 is local time when the NTP packet is

Re: unlock sys_sendsyslog

2021-03-17 Thread Alexander Bluhm
On Thu, Mar 11, 2021 at 12:40:37AM +0300, Vitaliy Makkoveev wrote: > Since UNIX domain sockets are unlocked it makes sense to unlock > sys_sendsyslog too. Console output still requires kernel lock to be > held but this path is only followed while `syslogf' socket is not set. > > New `syslogf_rwloc

ntpd offset loop refactoring

2021-03-18 Thread Alexander Bluhm
Hi, While hunting a bug in ntpd offset handling, I found some things that could be improved. Call the index of the offset loops "shift" consistently. Merge the two offset loops in client_update() into one. Assign the best value instead of memcpy. Use the same mechanism everywhere to avoid an inva

syslogd kernel timestamp

2021-03-18 Thread Alexander Bluhm
Hi, Since we stash log messages in the kernel, the timestamps added by syslogd are delayed. The kernel could add the timestamp when it receives the message by sendsyslog(2). This is more precise and can be expressed by more digits in the ISO timestamp. I have to copyin(9) at the beginning of se

/dev/kmem address range check

2021-03-22 Thread Alexander Bluhm
Hi, By specifying an invalid offset when reading /dev/kmem it is easy to crash the kernel from userland. sysctl kern.allowkmem=0 prevents this per default kernel: protection fault trap, code=0 Stopped at copyout+0x53: repe movsq (%rsi),%es:(%rdi) ddb> trace copyout() at copyout+0x53

Re: [External] : [ICMP] IP options lead to malformed reply

2021-03-26 Thread Alexander Bluhm
On Fri, Mar 26, 2021 at 11:00:22AM +, Schreilechner, Dominik wrote: > --- a/sys/netinet/ip_output.c > +++ b/sys/netinet/ip_output.c > @@ -765,6 +765,11 @@ ip_insertoptions(struct mbuf *m, struct mbuf *opt, int > *phlen) > optlen = opt->m_len - sizeof(p->ipopt_dst); > if (optlen

Re: update explicit_bzero test to not assume SIGSTKSZ to be constant

2021-03-26 Thread Alexander Bluhm
On Mon, Mar 22, 2021 at 08:38:23PM -0500, Brent Cook wrote: > In the next version of Linux glibc, SIGSTKSZ is defined at runtime if > source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to > bring in asprintf/vasprintf, which causes the explicit_bzero test to > fail to compile since

Re: Introduce fine grained pipex(4) locking

2022-07-14 Thread Alexander Bluhm
On Wed, Jul 13, 2022 at 12:49:53PM +0300, Vitaliy Makkoveev wrote: > Use per-session `pxs_mtx' mutex(9) to protect session context. Except > MPPE encryption, PPPOE sessions are mostly immutable, so no lock > required for that case. > > Global pipex(4) data is already protected by `pipex_list_mtx',

interface media current data

2022-07-14 Thread Alexander Bluhm
Hi, Instead of reading the unprotected pointer ifm_cur, drivers use API to access current media and data. New function ifmedia_current() provides read access with mutex locking. If anyone has some of these old network drivers, please test setting media type with ifconfig. ok? bluhm Index: arc

Re: Introduce fine grained pipex(4) locking

2022-07-15 Thread Alexander Bluhm
On Fri, Jul 15, 2022 at 02:05:12PM +0300, Vitaliy Makkoveev wrote: > I see no reason to move 'not_ours' checks outside pipex_common_input() > because this introduce code duplication. Also I see no reason to take > session lock after pipex_ppp_input() call, because it will be released > by caller ju

Re: Introduce fine grained pipex(4) locking

2022-07-15 Thread Alexander Bluhm
On Fri, Jul 15, 2022 at 03:52:28PM +0300, Vitaliy Makkoveev wrote: > I doubt this code duplication is better. Not sure. Let's commit your diff and figure out improvements later. OK bluhm@

Re: interface media current data

2022-07-15 Thread Alexander Bluhm
On Fri, Jul 15, 2022 at 02:05:40AM +0200, Alexander Bluhm wrote: > If anyone has some of these old network drivers, please test setting > media type with ifconfig. Updated diff. I have compile tested it on amd64, i386, octeon, macppc, sparc64. Setting media for gem(4) and bge(4) works.

Re: arp llinfo mutex

2022-07-16 Thread Alexander Bluhm
On Wed, Jun 29, 2022 at 10:17:26PM +0200, Martin Pieuchot wrote: > On 29/06/22(Wed) 19:40, Alexander Bluhm wrote: > Note that some times the code checks for the RTF_LLINFO flags and some > time for rt_llinfo != NULL. This is inconsistent and a bit confusing > now that we use a mute

Re: pf: pool for pf_anchor

2022-07-19 Thread Alexander Bluhm
On Tue, Jul 19, 2022 at 07:18:54PM +0200, Moritz Buhl wrote: > A solution would be to move the allocation of the pf_anchor struct > into a pool. One final question would be regarding the size of the > hiwat or hardlimit. Any suggestions? 10 seems way to low. We want to limit resource abuse. Bu

Re: pf: pool for pf_anchor

2022-07-20 Thread Alexander Bluhm
On Wed, Jul 20, 2022 at 09:43:01AM +0200, Moritz Buhl wrote: > Is the following diff OK? Did you run regress/sbin/pfctl ? OK bluhm@ > Index: sbin/pfctl/pfctl.c > === > RCS file: /mount/openbsd/cvs/src/sbin/pfctl/pfctl.c,v > retrievi

Re: wg: zap unused global

2022-07-21 Thread Alexander Bluhm
On Thu, Jul 21, 2022 at 11:18:23AM +, Klemens Nanni wrote: > Part of an old diff I still have in my tree. > > There since (second, proper) import in r1.3. > > OK? OK bluhm@ > Index: if_wg.c > === > RCS file: /cvs/src/sys/net/if

Re: timeout.9: dont' repeat TIMEOUT_PROC sentence

2022-07-22 Thread Alexander Bluhm
On Fri, Jul 22, 2022 at 10:38:32AM +, Klemens Nanni wrote: > timeout_set_flags() explains the flag and the next paragraph would simply > repeat it. > > timeout_set_proc() can be described simpler, just like what the > actual function does; this way I don't feel having just read the > same sen

Re: nd6: call nd6_timer() without argument

2022-07-22 Thread Alexander Bluhm
On Fri, Jul 22, 2022 at 11:18:14AM +, Klemens Nanni wrote: > nd6_timer_to is a global struct and nd6_timer() accesses it as such, > thereby ignoring its function argument. > > Make that clear when setting the timeout, which now goes like the other > two timeouts. > > OK? OK bluhm@ > Index:

Re: nd6: zap dead store nd6_allocated

2022-07-22 Thread Alexander Bluhm
On Fri, Jul 22, 2022 at 12:17:11PM +, Klemens Nanni wrote: > There since KAME IPv6 import in 1999. > > OK? Pool statistics has this info already. OK bluhm@ > diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c > index df6cf601b34..ff679bcb151 100644 > --- a/sys/netinet6/nd6.c > +++ b/sys/n

Re: Zap unused ND6_IS_LLINFO_PROBREACH and MAX_REACHABLE_TIME

2022-07-22 Thread Alexander Bluhm
On Fri, Jul 22, 2022 at 12:55:42PM +, Klemens Nanni wrote: > Leftovers from florian's RS/NA purge from the kernel in 2017. > > OK? My grep tells me that can be deleted. OK bluhm@ > diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h > index 82e674c5ecf..01ec60d02b9 100644 > --- a/sys/netin

Re: interface media current data

2022-07-22 Thread Alexander Bluhm
On Fri, Jul 15, 2022 at 04:51:25PM +0200, Alexander Bluhm wrote: > On Fri, Jul 15, 2022 at 02:05:40AM +0200, Alexander Bluhm wrote: > > If anyone has some of these old network drivers, please test setting > > media type with ifconfig. > > Updated diff. I have compile tes

Re: pipex(4): kill "Static" keyword

2022-07-22 Thread Alexander Bluhm
On Mon, Jul 18, 2022 at 12:31:47PM +0300, Vitaliy Makkoveev wrote: > We don't use "static" keyword for functions declaration to allow ddb(4) > debug. Also, many "Static" functions are called by pppx(4) layer outside > pipex(4) layer. > > This is the mostly mechanic diff, except the `pipex_pppoe_pa

Re: pppac(4): don't grab netlock within pppacioctl()

2022-07-22 Thread Alexander Bluhm
On Mon, Jul 18, 2022 at 01:50:37PM +0300, Vitaliy Makkoveev wrote: > pipex(4) doesn't rely on netlock anymore. OK bluhm@ > Index: sys/net/if_pppx.c > === > RCS file: /cvs/src/sys/net/if_pppx.c,v > retrieving revision 1.119 > diff -u

Re: interface media current data

2022-07-22 Thread Alexander Bluhm
On Fri, Jul 22, 2022 at 05:44:52PM +0200, Mark Kettenis wrote: > > Date: Fri, 22 Jul 2022 17:01:20 +0200 > > From: Alexander Bluhm > > > > On Fri, Jul 15, 2022 at 04:51:25PM +0200, Alexander Bluhm wrote: > > > On Fri, Jul 15, 2022 at 02:05:40AM +0200, Alexander B

Re: pf: DIOCXCOMMIT and copyin

2022-07-22 Thread Alexander Bluhm
On Thu, Jul 21, 2022 at 11:13:28AM +0200, Moritz Buhl wrote: > Hi tech, > > for the other two DIOCX ioctls syzkaller showed that it is possible > to grab netlock while doing copyin. > > The same problem should exist for DIOCXCOMMIT but syzkaller didn't > find it yet. > > In case anybody can repr

ipv6 local deliver net lock

2022-07-22 Thread Alexander Bluhm
Hi, During regress testing I found this bug. splassert: rip6_input: want 1 have 2 Starting stack trace... rip6_input(1,2,d0c6b7ad,f57ff9fc) at rip6_input+0x166 rip6_input(f57ffbfc,f57ffbe8,3a,18) at rip6_input+0x166 icmp6_input(f57ffbfc,f57ffbe8,3a,18) at icmp6_input+0x66d ip_deliver(f57ffbfc,f57

ipv4 reassemble in parallel

2022-07-22 Thread Alexander Bluhm
Hi, The IPv6 reassembly code looks MP safe. So we can run it in parallel. Note that ip_ours() runs with shared netlock, while ip_local() has exclusive netlock after queuing. ok? bluhm Index: netinet/ip_input.c === RCS file: /data/

OpenBSD Errata: July 24, 2022 (xserver cron)

2022-07-24 Thread Alexander Bluhm
Errata patches for X11 server and cron daemon have been released for OpenBSD 7.0 and 7.1. Binary updates for the amd64, i386 and arm64 platform are available via the syspatch utility. Source code patches can be found on the respective errata page: https://www.openbsd.org/errata71.html https:

  1   2   3   4   5   6   7   8   9   10   >