NetBSD Problem Report #54150
From paul@whooppee.com Thu May 2 11:06:45 2019
Return-Path: <paul@whooppee.com>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id A0EA77A186
for <gnats-bugs@gnats.NetBSD.org>; Thu, 2 May 2019 11:06:45 +0000 (UTC)
Message-Id: <20190502110642.1BA5530F2C2@speedy.whooppee.com>
Date: Thu, 2 May 2019 19:06:42 +0800 (PST)
From: paul@whooppee.com
Reply-To: paul@whooppee.com
To: gnats-bugs@NetBSD.org
Subject: COMPAT_50 vs NET_RT_IFLIST
X-Send-Pr-Version: 3.95
>Number: 54150
>Category: kern
>Synopsis: COMPAT_50 vs NET_RT_IFLIST broken
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu May 02 11:10:00 +0000 2019
>Last-Modified: Tue Apr 21 01:50:01 +0000 2020
>Originator: Paul Goyette
>Release: NetBSD 8.99.37
>Organization:
+--------------------+--------------------------+-----------------------+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org |
+--------------------+--------------------------+-----------------------+
>Environment:
System: NetBSD speedy.whooppee.com 8.99.37 NetBSD 8.99.37 (SPEEDY 2019-04-24 23:45:06 UTC) #0: Thu Apr 25 09:00:09 UTC 2019 paul@speedy.whooppee.com:/build/netbsd-local/obj/amd64/sys/arch/amd64/compile/SPEEDY amd64
Architecture: x86_64
Machine: amd64
>Description:
Recent kernels are unable to properly return results for a NetBSD-5.2
version of getifaddrs.
>How-To-Repeat:
1. Build a release, and install it (qemu VM is fine)
2. Create a /chroot52 directory, and unpack the base.tgz from
NetBSD-5.2 into that directory
3. Boot the result, login as root, and execute the command
# chroot /chroot52 ifconfig -l
4. A working system will display lo0 (and for qemu, wm0) while a
broken system displays a blank line.
A manual bisect of the issue shows that the compat_50 code was working
correctly as of 2019-09-21 at 10:00:00 UTC, while kernels from 19:18:10
on that same date fail. During that interval there was a series of
relevant commits starting with
Module Name: src
Committed By: roy
Date: Wed Sep 21 10:50:23 UTC 2016
Modified Files:
src/share/man/man4: route.4
src/sys/compat/common: Makefile
src/sys/compat/net: if.h route.h
src/sys/net: if.h route.h rtsock.c
src/sys/rump/net/lib/libnet: Makefile
src/sys/sys: socket.h
Added Files:
src/sys/compat/common: rtsock_70.c
Log Message:
Add ifam_pid and ifam_addrflags to ifa_msghdr.
Re-version RTM_NEWADDR, RTM_DELADDR, RTM_CHGADDR and NET_RT_IFLIST.
Add compat code for old version.
>Fix:
>Audit-Trail:
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Fri, 03 May 2019 02:48:00 +0700
I think I understand what might be happening here now.
getifaddrs() uses sysctl(net.route.0.0.iflist.0)
That sysctl returns a list of message structures, when we are
running compatible userland/kernel there will be a sequence of
RTM_IFINFO ad RTM_NEWADDR messages.
When userland is NetBSD 5, the RTM_IFINFO is replaced by RTM_OOIFINFO
(which is generated by compat_50_iflist) and RTM_ONEWADDR (which is
generated by compat_70_iflist_addr)
It is compat_70_iflist_addr as this structure had not changed between
netbsd 5 and netbsd 7 (the change between netbsd 7 and 8 was the first
change of that struct)
Of itself, all that is OK. However, there is another change. Each
message is preceded by a header, which starts with a length, type,
and version. The length and type are handled by the code above.
Unfortunately, RTM_VERSION altered between NetBSD 5 and NetBSD 7,
compat_50_iflist() generates a message rith RTM_OVERSION (3) in it, and
compat_70_iflist_addr() generates a message with RTM_VERSION (4).
Those are the appropriate values for NetBSD 5 and 7 resp.
But both of those end up appearing in the same sysctl result.
getifaddrs() (all versions - or at least, the NetBSD 5 and later ones,
which is all Ihave looked at) checks that the version field is
RTM_VERSION (whichever version is appropriate for the version of
getifaddrs() in use) and ignores anything that is not.
The NEWADDR messages, which appear as RTMVersion 4 all get ignored by
a NetBSD 5 getifaddrs(), and those are what contain the interface names
(plus more).
I suspect that what we need is to make a compat_50_iflist_addr()
function, identical to compat_70_iflist_addr() except using RTM_OVERSION
instead of RTM_VERSION (the two could use some common routine to fill in
all the rest of the details). However, I have not yet tried it.
kre
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Fri, 03 May 2019 10:56:36 +0700
Ignore most of my last message ... or treat it as what happens
when running on NetBSD 8 (which I have not tested, but is probably
close). Since this was supposed to be the same from pre NetBSD-8
through HEAD, I was testing on my laptop which is running a kernel
from just before PaulG's module merge (January 6 or something).
With that, what I described is approx what happens, I think.
After the merge, the sysctl messages are all the same version - but
the wrong one (a -5 binary generates RTM_VERSION 4 messages, and
should be 3). I have a hack fix for that, after which ifconfig -l
from -5 core dumps (aborts) which is the same as my test program
(new sources, doing the NetBSD 5 sysctl, so I can see what is
actually being returned). (The abort() tells me that at least
the getifaddrs() code is not simply ignoring everything returned,
which is what happens when the version is wrong).
The abort() comes from in getifaddrs() from the line ...
case RTM_NEWADDR:
ifam = (struct ifa_msghdr *)(void *)rtm;
if (idx && ifam->ifam_index != idx)
abort(); /* this cannot happen */
I have no idea what the comment is about, ifam->ifam_index comes from
the sysctl result, idx is calculated in getifaddrs() - what it should
be saying is "this should not happen" - as obviously it can, if the
kernel is returning messages in a form that getifaddrs() is not
expecting. An abort for that, rather than just a failure is a little
severe, I think.
Note that the second copy of almost the same code should be OK, as when
that runs, the first one must have already passed, and so there, "cannot
happen" really should be true (absent hardware or compiler issues.)
Still looking, but clearly something is not right about the message
formats (it does not appear to be an "O" counting error ... but who
knows).
kre
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc: Paul Goyette <paul@whooppee.com>, Roy Marples <roy@marples.name>,
Matthew Green <mrg@netbsd.org>
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Fri, 03 May 2019 11:46:26 +0700
OK ... the patch below seems to fix this one - for the specific case
of NetBSD 5 binaries running on a -current kernel (a patch for -8 would
be quite different, but along the same lines).
Whether this correctly handles older (older than 1.4?) binaries, or even
NetBSD 7 (or 8) binaries I have not tested. But it should not be worse
than it was, I think.
The primary problems were the incorrect RTM_VERSION in the messages,
and that struct ifa_msghdr has a different layout (field order changed)
in NetBSD 5 & NetBSD 7 - so returning a NetBSD 7 format struct to a
NetBSD 5 client does not work well.
I'd appreciate it if everyone who has looked at this problem could
take a look at this, and send comments before it gets committed.
(Hence I have added a few cc's to this message, so people can get
the patch in a non-gnats-mangled format ... anyone else who wants
that send me a message off list).
I am particularly wary of the RTS_CTASSERT() stuff in the new
rt_getvers() function - that function is more or less a clone of
rt_getlen() (which has these same RTS_CTASSERTs) and seems to
work ok, on amd64 anyway. Whether this will work on i386 (and
other 32 bit systems) I have no idea.
The companion PR (for "route monitor") I have not yet investigated
(except to discover that it is not as simple as I thought it might be.)
kre
ps: IMNSHO, way too many files need to be touched to add a new hook.
I would suggest that we need a better way, where it just needs to be
defined (one place) and then used (wherever needed) and nothing else
needs to be manually edited - all the bookkeeping should be handled
automatically.
Index: compat/common/rtsock_50.c
===================================================================
RCS file: /cvsroot/src/sys/compat/common/rtsock_50.c,v
retrieving revision 1.13
diff -u -r1.13 rtsock_50.c
--- compat/common/rtsock_50.c 29 Apr 2019 16:12:30 -0000 1.13
+++ compat/common/rtsock_50.c 3 May 2019 04:26:38 -0000
@@ -151,6 +151,28 @@
return 0;
}
+int
+compat_50_iflist_addr(struct rt_walkarg *w, struct ifaddr *ifa,
+ struct rt_addrinfo *info)
+{
+ int len, error;
+
+ if ((error = rt_msg3(RTM_ONEWADDR, info, 0, w, &len)))
+ return error;
+ if (w->w_where && w->w_tmem && w->w_needed <= 0) {
+ struct ifa_msghdr50 *ifam;
+
+ ifam = (struct ifa_msghdr50 *)w->w_tmem;
+ ifam->ifam_index = ifa->ifa_ifp->if_index;
+ ifam->ifam_flags = ifa->ifa_flags;
+ ifam->ifam_metric = ifa->ifa_metric;
+ ifam->ifam_addrs = info->rti_addrs;
+ if ((error = copyout(w->w_tmem, w->w_where, len)) == 0)
+ w->w_where = (char *)w->w_where + len;
+ }
+ return error;
+}
+
void
rtsock_50_init(void)
{
@@ -170,6 +192,8 @@
compat_50_rt_ifannouncemsg);
MODULE_HOOK_SET(rtsock_rt_ieee80211msg_50_hook, "rts_50",
compat_50_rt_ieee80211msg);
+ MODULE_HOOK_SET(rtsock_iflist_addr_50_hook, "rts_50",
+ compat_50_iflist_addr);
sysctl_net_route_setup(&clog, PF_OROUTE, "ortable");
}
@@ -187,4 +211,5 @@
MODULE_HOOK_UNSET(rtsock_rt_addrmsg_50_hook);
MODULE_HOOK_UNSET(rtsock_rt_ifannouncemsg_50_hook);
MODULE_HOOK_UNSET(rtsock_rt_ieee80211msg_50_hook);
+ MODULE_HOOK_UNSET(rtsock_iflist_addr_50_hook);
}
Index: compat/net/if.h
===================================================================
RCS file: /cvsroot/src/sys/compat/net/if.h,v
retrieving revision 1.4
diff -u -r1.4 if.h
--- compat/net/if.h 21 Sep 2016 10:50:23 -0000 1.4
+++ compat/net/if.h 3 May 2019 04:26:38 -0000
@@ -171,6 +171,9 @@
int ifam_metric; /* value of ifa_metric */
};
+int compat_50_iflist_addr(struct rt_walkarg *, struct ifaddr *,
+ struct rt_addrinfo *);
+
/*
* Message format announcing the arrival or departure of a network interface.
*/
Index: kern/compat_stub.c
===================================================================
RCS file: /cvsroot/src/sys/kern/compat_stub.c,v
retrieving revision 1.11
diff -u -r1.11 compat_stub.c
--- kern/compat_stub.c 29 Apr 2019 16:12:30 -0000 1.11
+++ kern/compat_stub.c 3 May 2019 04:26:46 -0000
@@ -207,6 +207,7 @@
struct rtsock_rt_addrmsg_src_50_hook_t rtsock_rt_addrmsg_src_50_hook;
struct rtsock_rt_addrmsg_50_hook_t rtsock_rt_addrmsg_50_hook;
struct rtsock_rt_ieee80211msg_50_hook_t rtsock_rt_ieee80211msg_50_hook;
+struct rtsock_iflist_addr_50_hook_t rtsock_iflist_addr_50_hook;
/*
* rtsock 70 compatability
Index: net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.249
diff -u -r1.249 rtsock.c
--- net/rtsock.c 29 Apr 2019 05:42:09 -0000 1.249
+++ net/rtsock.c 3 May 2019 04:26:49 -0000
@@ -364,9 +364,12 @@
error = sysctl_iflist_addr(w, ifa, &info);
break;
case NET_RT_OIFLIST:
+ MODULE_HOOK_CALL(rtsock_iflist_70_hook,
+ (w, ifa, &info), enosys(), error);
+ break;
case NET_RT_OOIFLIST:
case NET_RT_OOOIFLIST:
- MODULE_HOOK_CALL(rtsock_iflist_70_hook,
+ MODULE_HOOK_CALL(rtsock_iflist_addr_50_hook,
(w, ifa, &info), enosys(), error);
break;
default:
Index: net/rtsock_shared.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock_shared.c,v
retrieving revision 1.8
diff -u -r1.8 rtsock_shared.c
--- net/rtsock_shared.c 29 Apr 2019 16:12:30 -0000 1.8
+++ net/rtsock_shared.c 3 May 2019 04:26:50 -0000
@@ -1159,6 +1159,67 @@
}
+static int
+rt_getvers(int type)
+{
+ RTS_CTASSERT(__alignof(struct ifa_msghdr) >= sizeof(uint64_t));
+ RTS_CTASSERT(__alignof(struct if_msghdr) >= sizeof(uint64_t));
+ RTS_CTASSERT(__alignof(struct if_announcemsghdr) >= sizeof(uint64_t));
+ RTS_CTASSERT(__alignof(struct rt_msghdr) >= sizeof(uint64_t));
+
+ switch (type) {
+ case RTM_ODELADDR:
+ case RTM_ONEWADDR:
+ case RTM_OCHGADDR:
+ if (rtsock_iflist_70_hook.hooked)
+ return RTM_OVERSION;
+ else {
+#ifdef RTSOCK_DEBUG
+ printf("%s: unsupported RTM type %d\n", __func__, type);
+#endif
+ return -1;
+ }
+
+ case RTM_DELADDR:
+ case RTM_NEWADDR:
+ case RTM_CHGADDR:
+ return RTM_VERSION;
+
+ case RTM_OOIFINFO:
+ if (rtsock_iflist_14_hook.hooked)
+ return RTM_OVERSION;
+ else {
+#ifdef RTSOCK_DEBUG
+ printf("%s: unsupported RTM type RTM_OOIFINFO\n",
+ __func__);
+#endif
+ return -1;
+ }
+
+ case RTM_OIFINFO:
+ if (rtsock_iflist_50_hook.hooked)
+ return RTM_OVERSION;
+ else {
+#ifdef RTSOCK_DEBUG
+ printf("%s: unsupported RTM type RTM_OIFINFO\n",
+ __func__);
+#endif
+ return -1;
+ }
+
+ case RTM_IFINFO:
+ return RTM_VERSION;
+
+ case RTM_IFANNOUNCE:
+ case RTM_IEEE80211:
+ return RTM_VERSION;
+
+ default:
+ return RTM_XVERSION;
+ }
+}
+
+
struct mbuf *
COMPATNAME(rt_msg1)(int type, struct rt_addrinfo *rtinfo, void *data, int datalen)
{
@@ -1212,7 +1273,7 @@
if (m->m_pkthdr.len != len)
goto out;
rtm->rtm_msglen = len;
- rtm->rtm_version = RTM_XVERSION;
+ rtm->rtm_version = rt_getvers(type);
rtm->rtm_type = type;
return m;
out:
@@ -1290,7 +1351,7 @@
if (cp) {
struct rt_xmsghdr *rtm = (struct rt_xmsghdr *)cp0;
- rtm->rtm_version = RTM_XVERSION;
+ rtm->rtm_version = rt_getvers(type);
rtm->rtm_type = type;
rtm->rtm_msglen = len;
}
Index: sys/compat_stub.h
===================================================================
RCS file: /cvsroot/src/sys/sys/compat_stub.h,v
retrieving revision 1.15
diff -u -r1.15 compat_stub.h
--- sys/compat_stub.h 29 Apr 2019 16:12:30 -0000 1.15
+++ sys/compat_stub.h 3 May 2019 04:26:51 -0000
@@ -269,6 +269,8 @@
MODULE_HOOK(rtsock_rt_ifannouncemsg_50_hook, void, (struct ifnet *, int));
MODULE_HOOK(rtsock_rt_ieee80211msg_50_hook, void,
(struct ifnet *, int, void *, size_t));
+MODULE_HOOK(rtsock_iflist_addr_50_hook, int,
+ (struct rt_walkarg *, struct ifaddr *, struct rt_addrinfo *));
/*
* Hooks for rtsock_70
From: Paul Goyette <paul@whooppee.com>
To: Robert Elz <kre@munnari.OZ.AU>
Cc: gnats-bugs@netbsd.org, Roy Marples <roy@marples.name>,
Matthew Green <mrg@netbsd.org>
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Fri, 3 May 2019 13:16:14 +0800 (PST)
I think that the new hook function compat_50_iflist_addr() could be
declared static, and thus you wouldn't need to add it to net/if.h
But otherwise it looks reasonable to me.
On Fri, 3 May 2019, Robert Elz wrote:
> OK ... the patch below seems to fix this one - for the specific case
> of NetBSD 5 binaries running on a -current kernel (a patch for -8 would
> be quite different, but along the same lines).
>
> Whether this correctly handles older (older than 1.4?) binaries, or even
> NetBSD 7 (or 8) binaries I have not tested. But it should not be worse
> than it was, I think.
>
> The primary problems were the incorrect RTM_VERSION in the messages,
> and that struct ifa_msghdr has a different layout (field order changed)
> in NetBSD 5 & NetBSD 7 - so returning a NetBSD 7 format struct to a
> NetBSD 5 client does not work well.
>
> I'd appreciate it if everyone who has looked at this problem could
> take a look at this, and send comments before it gets committed.
> (Hence I have added a few cc's to this message, so people can get
> the patch in a non-gnats-mangled format ... anyone else who wants
> that send me a message off list).
>
> I am particularly wary of the RTS_CTASSERT() stuff in the new
> rt_getvers() function - that function is more or less a clone of
> rt_getlen() (which has these same RTS_CTASSERTs) and seems to
> work ok, on amd64 anyway. Whether this will work on i386 (and
> other 32 bit systems) I have no idea.
>
> The companion PR (for "route monitor") I have not yet investigated
> (except to discover that it is not as simple as I thought it might be.)
>
> kre
>
> ps: IMNSHO, way too many files need to be touched to add a new hook.
>
> I would suggest that we need a better way, where it just needs to be
> defined (one place) and then used (wherever needed) and nothing else
> needs to be manually edited - all the bookkeeping should be handled
> automatically.
>
>
> Index: compat/common/rtsock_50.c
> ===================================================================
> RCS file: /cvsroot/src/sys/compat/common/rtsock_50.c,v
> retrieving revision 1.13
> diff -u -r1.13 rtsock_50.c
> --- compat/common/rtsock_50.c 29 Apr 2019 16:12:30 -0000 1.13
> +++ compat/common/rtsock_50.c 3 May 2019 04:26:38 -0000
> @@ -151,6 +151,28 @@
> return 0;
> }
>
> +int
> +compat_50_iflist_addr(struct rt_walkarg *w, struct ifaddr *ifa,
> + struct rt_addrinfo *info)
> +{
> + int len, error;
> +
> + if ((error = rt_msg3(RTM_ONEWADDR, info, 0, w, &len)))
> + return error;
> + if (w->w_where && w->w_tmem && w->w_needed <= 0) {
> + struct ifa_msghdr50 *ifam;
> +
> + ifam = (struct ifa_msghdr50 *)w->w_tmem;
> + ifam->ifam_index = ifa->ifa_ifp->if_index;
> + ifam->ifam_flags = ifa->ifa_flags;
> + ifam->ifam_metric = ifa->ifa_metric;
> + ifam->ifam_addrs = info->rti_addrs;
> + if ((error = copyout(w->w_tmem, w->w_where, len)) == 0)
> + w->w_where = (char *)w->w_where + len;
> + }
> + return error;
> +}
> +
> void
> rtsock_50_init(void)
> {
> @@ -170,6 +192,8 @@
> compat_50_rt_ifannouncemsg);
> MODULE_HOOK_SET(rtsock_rt_ieee80211msg_50_hook, "rts_50",
> compat_50_rt_ieee80211msg);
> + MODULE_HOOK_SET(rtsock_iflist_addr_50_hook, "rts_50",
> + compat_50_iflist_addr);
> sysctl_net_route_setup(&clog, PF_OROUTE, "ortable");
> }
>
> @@ -187,4 +211,5 @@
> MODULE_HOOK_UNSET(rtsock_rt_addrmsg_50_hook);
> MODULE_HOOK_UNSET(rtsock_rt_ifannouncemsg_50_hook);
> MODULE_HOOK_UNSET(rtsock_rt_ieee80211msg_50_hook);
> + MODULE_HOOK_UNSET(rtsock_iflist_addr_50_hook);
> }
> Index: compat/net/if.h
> ===================================================================
> RCS file: /cvsroot/src/sys/compat/net/if.h,v
> retrieving revision 1.4
> diff -u -r1.4 if.h
> --- compat/net/if.h 21 Sep 2016 10:50:23 -0000 1.4
> +++ compat/net/if.h 3 May 2019 04:26:38 -0000
> @@ -171,6 +171,9 @@
> int ifam_metric; /* value of ifa_metric */
> };
>
> +int compat_50_iflist_addr(struct rt_walkarg *, struct ifaddr *,
> + struct rt_addrinfo *);
> +
> /*
> * Message format announcing the arrival or departure of a network interface.
> */
> Index: kern/compat_stub.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/compat_stub.c,v
> retrieving revision 1.11
> diff -u -r1.11 compat_stub.c
> --- kern/compat_stub.c 29 Apr 2019 16:12:30 -0000 1.11
> +++ kern/compat_stub.c 3 May 2019 04:26:46 -0000
> @@ -207,6 +207,7 @@
> struct rtsock_rt_addrmsg_src_50_hook_t rtsock_rt_addrmsg_src_50_hook;
> struct rtsock_rt_addrmsg_50_hook_t rtsock_rt_addrmsg_50_hook;
> struct rtsock_rt_ieee80211msg_50_hook_t rtsock_rt_ieee80211msg_50_hook;
> +struct rtsock_iflist_addr_50_hook_t rtsock_iflist_addr_50_hook;
>
> /*
> * rtsock 70 compatability
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvsroot/src/sys/net/rtsock.c,v
> retrieving revision 1.249
> diff -u -r1.249 rtsock.c
> --- net/rtsock.c 29 Apr 2019 05:42:09 -0000 1.249
> +++ net/rtsock.c 3 May 2019 04:26:49 -0000
> @@ -364,9 +364,12 @@
> error = sysctl_iflist_addr(w, ifa, &info);
> break;
> case NET_RT_OIFLIST:
> + MODULE_HOOK_CALL(rtsock_iflist_70_hook,
> + (w, ifa, &info), enosys(), error);
> + break;
> case NET_RT_OOIFLIST:
> case NET_RT_OOOIFLIST:
> - MODULE_HOOK_CALL(rtsock_iflist_70_hook,
> + MODULE_HOOK_CALL(rtsock_iflist_addr_50_hook,
> (w, ifa, &info), enosys(), error);
> break;
> default:
> Index: net/rtsock_shared.c
> ===================================================================
> RCS file: /cvsroot/src/sys/net/rtsock_shared.c,v
> retrieving revision 1.8
> diff -u -r1.8 rtsock_shared.c
> --- net/rtsock_shared.c 29 Apr 2019 16:12:30 -0000 1.8
> +++ net/rtsock_shared.c 3 May 2019 04:26:50 -0000
> @@ -1159,6 +1159,67 @@
> }
>
>
> +static int
> +rt_getvers(int type)
> +{
> + RTS_CTASSERT(__alignof(struct ifa_msghdr) >= sizeof(uint64_t));
> + RTS_CTASSERT(__alignof(struct if_msghdr) >= sizeof(uint64_t));
> + RTS_CTASSERT(__alignof(struct if_announcemsghdr) >= sizeof(uint64_t));
> + RTS_CTASSERT(__alignof(struct rt_msghdr) >= sizeof(uint64_t));
> +
> + switch (type) {
> + case RTM_ODELADDR:
> + case RTM_ONEWADDR:
> + case RTM_OCHGADDR:
> + if (rtsock_iflist_70_hook.hooked)
> + return RTM_OVERSION;
> + else {
> +#ifdef RTSOCK_DEBUG
> + printf("%s: unsupported RTM type %d\n", __func__, type);
> +#endif
> + return -1;
> + }
> +
> + case RTM_DELADDR:
> + case RTM_NEWADDR:
> + case RTM_CHGADDR:
> + return RTM_VERSION;
> +
> + case RTM_OOIFINFO:
> + if (rtsock_iflist_14_hook.hooked)
> + return RTM_OVERSION;
> + else {
> +#ifdef RTSOCK_DEBUG
> + printf("%s: unsupported RTM type RTM_OOIFINFO\n",
> + __func__);
> +#endif
> + return -1;
> + }
> +
> + case RTM_OIFINFO:
> + if (rtsock_iflist_50_hook.hooked)
> + return RTM_OVERSION;
> + else {
> +#ifdef RTSOCK_DEBUG
> + printf("%s: unsupported RTM type RTM_OIFINFO\n",
> + __func__);
> +#endif
> + return -1;
> + }
> +
> + case RTM_IFINFO:
> + return RTM_VERSION;
> +
> + case RTM_IFANNOUNCE:
> + case RTM_IEEE80211:
> + return RTM_VERSION;
> +
> + default:
> + return RTM_XVERSION;
> + }
> +}
> +
> +
> struct mbuf *
> COMPATNAME(rt_msg1)(int type, struct rt_addrinfo *rtinfo, void *data, int datalen)
> {
> @@ -1212,7 +1273,7 @@
> if (m->m_pkthdr.len != len)
> goto out;
> rtm->rtm_msglen = len;
> - rtm->rtm_version = RTM_XVERSION;
> + rtm->rtm_version = rt_getvers(type);
> rtm->rtm_type = type;
> return m;
> out:
> @@ -1290,7 +1351,7 @@
> if (cp) {
> struct rt_xmsghdr *rtm = (struct rt_xmsghdr *)cp0;
>
> - rtm->rtm_version = RTM_XVERSION;
> + rtm->rtm_version = rt_getvers(type);
> rtm->rtm_type = type;
> rtm->rtm_msglen = len;
> }
> Index: sys/compat_stub.h
> ===================================================================
> RCS file: /cvsroot/src/sys/sys/compat_stub.h,v
> retrieving revision 1.15
> diff -u -r1.15 compat_stub.h
> --- sys/compat_stub.h 29 Apr 2019 16:12:30 -0000 1.15
> +++ sys/compat_stub.h 3 May 2019 04:26:51 -0000
> @@ -269,6 +269,8 @@
> MODULE_HOOK(rtsock_rt_ifannouncemsg_50_hook, void, (struct ifnet *, int));
> MODULE_HOOK(rtsock_rt_ieee80211msg_50_hook, void,
> (struct ifnet *, int, void *, size_t));
> +MODULE_HOOK(rtsock_iflist_addr_50_hook, int,
> + (struct rt_walkarg *, struct ifaddr *, struct rt_addrinfo *));
>
> /*
> * Hooks for rtsock_70
>
>
>
> !DSPAM:5ccbc7c0112531254785785!
>
>
+--------------------+--------------------------+-----------------------+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org |
+--------------------+--------------------------+-----------------------+
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Fri, 03 May 2019 18:57:50 +0700
Note that while the patch supplied makes (a NetBSD 5) "ifconfig -l" work,
(and also makes NetBSD 5 "route -d monitor" work, or at least not do
nothing ... note, the "-d" causes this to be completely different than
"route monitor") it is not sufficient to make NetBSD 5 ifconfig work
comnpletely (not even just to display interface status).
Some of the info from the interface is available, and all interfaces
are located (or so it seems) but no IP addresses (v4 or v6) are listed.
link layer addresses are, as are capabilities, and the interface
header line (the one that contains flags, UP, mtu, ...)
I shall continue looking...
kre
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Sun, 05 May 2019 08:55:08 +0700
Date: Fri, 3 May 2019 12:00:02 +0000 (UTC)
From: Robert Elz <kre@munnari.OZ.AU>
Message-ID: <20190503120002.A0E4E7A1D3@mollari.NetBSD.org>
| it is not sufficient to make NetBSD 5 ifconfig work
| comnpletely (not even just to display interface status).
This looks to be an alignment problem - the sockaddr's containing
the messages in the NEWADDR messages are aligned to a bigger boundary (8)
than they should be for the NetBSD 5 format (4).
Still looking for the best way to fix this.
Note that while looking at the interface's config this way is still
failing, the NetBSD 5 ifconfig can configure a HEAD system just fine
(or at least all I tried, which admittedly was not a lot).
kre
From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, paul@whooppee.com
Subject: re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Mon, 06 May 2019 06:35:42 +1000
> This looks to be an alignment problem - the sockaddr's containing
> the messages in the NEWADDR messages are aligned to a bigger boundary (8)
> than they should be for the NetBSD 5 format (4).
this might be the original thing i posted about:
#if !defined(_KERNEL) || !defined(COMPAT_RTSOCK)
#define __align64 __aligned(sizeof(uint64_t))
...
struct if_msghdr {
u_short ifm_msglen __align64;
...
struct if_data ifm_data __align64;
...
this means that compat code aaccessing the *real* one has the
wrong alignment applied to the these members.
i'm pretty sure all the "__align64" in sys/net/*.h are wrong.
.mrg.
From: Robert Elz <kre@munnari.OZ.AU>
To: matthew green <mrg@eterna.com.au>
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, paul@whooppee.com
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Mon, 06 May 2019 07:39:10 +0700
Date: Mon, 06 May 2019 06:35:42 +1000
From: matthew green <mrg@eterna.com.au>
Message-ID: <29305.1557088542@splode.eterna.com.au>
Thanks for the push, I had been letting this slide a bit for the past
day or so.
| this might be the original thing i posted about:
| i'm pretty sure all the "__align64" in sys/net/*.h are wrong.
You're probably right, but some of those have already gone away (in
the code I am using)
The problem is, I think (in rt_msg2() in net/rtsock_shared.c):
if ((len = rt_getlen(type)) == -1)
where rt_getlen() does:
switch (type) {
case RTM_ODELADDR:
case RTM_ONEWADDR:
case RTM_OCHGADDR:
if (rtsock_iflist_70_hook.hooked)
return sizeof(struct ifa_msghdr70);
which happens even when the actual struct that is being placed there
is ifa_msghdr50 (which doesn't have the align, whereas ifa_msggdr70
still does. That, and the order of the final 2 fields, is the difference
between the two.)
If we could be confident that ifa_msghdr70 doesn't need the __align64
then that might fix things fairly easily. I haven't really been
concentrating on making NetBSD 7 binaries work, as we have not had
reports that they don't (and NetBSD 7 binaries on a NetBSD8 kernel
would be a much more common thing - particularly soon after the
message formats were changed).
However, the __aligns are in net/if.h 1.174 (the branch point for
netbsd-7) so I suspect that they really are needed for that case.
They were added in 1.150 which is where PF_ROUTE was changed
(to make 64 bit clean versions).
I guess I should try (a variant of) the current patch reconstructed
for -8 - it may be that this alignment problem is only one in HEAD
because of the way the modules are built, and that with the other
issues fixed, -8 would work.
Rather I have been seeing if I can find some way to reasonably change
that sizeof to use the correct struct - the one that is actually going
to be put in the space being left for it.
I think that's hoing to need changing rt_getlen() as just the message
type is not enough info to know - nor can we just look at which compat
fiunctions are loaded into the kernel (the way the current code does)
as whenever we have compat50 we also have compat70, and having compat50
loaded does not mean that the current binary wants it. The entry
code knows which protocol is in use (PF_ROUTE or PF_OROUTE) and just
needs to pass that down I think - then rt_getlen() could return the
appropriate length, rather than the wrong one.
This is all just a matter of bookkeeping somewhere, I am just (slowly)
looking to see if I can make the right info available in the right place
in a way that doesn't completely break the way the current compat
modules are built & used.
kre
ps: I am also not sure that the conversion of the timeval from 32 bit
time_t to 64 bit is being handled correctly, but I also don't know of
anything that actually uses that data.
From: "Greg A. Woods" <woods@planix.ca>
To: NetBSD GNATS <gnats-bugs@NetBSD.org>
Cc:
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Mon, 20 Apr 2020 18:22:09 -0700
--pgp-sign-Multipart_Mon_Apr_20_18:22:01_2020-1
Content-Type: text/plain; charset=US-ASCII
I just tried a newish kernel (8.99.32) on a 5.1 i386 system and quickly
discovered that the network didn't start:
lilbit# ifconfig -a
ifconfig: getifaddrs: No such file or directory
Am I correct in guessing this (kern/54150) is the same issue?
--
Greg A. Woods <gwoods@acm.org>
Kelowna, BC +1 250 762-7675 RoboHack <woods@robohack.ca>
Planix, Inc. <woods@planix.com> Avoncote Farms <woods@avoncote.ca>
--pgp-sign-Multipart_Mon_Apr_20_18:22:01_2020-1
Content-Type: application/pgp-signature
Content-Transfer-Encoding: 7bit
Content-Description: OpenPGP Digital Signature
-----BEGIN PGP SIGNATURE-----
iF0EABECAB0WIQRuK6dmwVAucmRxuh9mfXG3eL/0fwUCXp5KuwAKCRBmfXG3eL/0
f+NEAKDfAhOtQatYI7+6S/909qq0/EiBugCg4wPVvd/lRiUargFFgGN+IL155u0=
=E1QH
-----END PGP SIGNATURE-----
--pgp-sign-Multipart_Mon_Apr_20_18:22:01_2020-1--
From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: "Greg A. Woods" <woods@planix.ca>
Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
Date: Mon, 20 Apr 2020 18:46:56 -0700 (PDT)
On Tue, 21 Apr 2020, Greg A. Woods wrote:
> The following reply was made to PR kern/54150; it has been noted by GNATS.
>
> From: "Greg A. Woods" <woods@planix.ca>
> To: NetBSD GNATS <gnats-bugs@NetBSD.org>
> Cc:
> Subject: Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST
> Date: Mon, 20 Apr 2020 18:22:09 -0700
>
> --pgp-sign-Multipart_Mon_Apr_20_18:22:01_2020-1
> Content-Type: text/plain; charset=US-ASCII
>
> I just tried a newish kernel (8.99.32) on a 5.1 i386 system and quickly
> discovered that the network didn't start:
>
> lilbit# ifconfig -a
> ifconfig: getifaddrs: No such file or directory
>
> Am I correct in guessing this (kern/54150) is the same issue?
Most likely yes.
+--------------------+--------------------------+-----------------------+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org |
+--------------------+--------------------------+-----------------------+
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home