Date: Fri, 19 Feb 2021 16:24:09 +0100 From: "Kristof Provost" <kp@FreeBSD.org> To: "Alexander V. Chernikov" <melifaro@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 2fe5a79425c7 - main - Fix dst/netmask handling in routing socket code. Message-ID: <26E2BA35-291E-4DC5-BDCB-D98347D7E24C@FreeBSD.org> In-Reply-To: <202102162031.11GKV0T6060307@gitrepo.freebsd.org> References: <202102162031.11GKV0T6060307@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 16 Feb 2021, at 21:31, Alexander V. Chernikov wrote: > The branch main has been updated by melifaro: > > URL: = > https://cgit.FreeBSD.org/src/commit/?id=3D2fe5a79425c79f7b828acd91da66d= 97230925fc8 > > commit 2fe5a79425c79f7b828acd91da66d97230925fc8 > Author: Alexander V. Chernikov <melifaro@FreeBSD.org> > AuthorDate: 2021-02-16 20:30:04 +0000 > Commit: Alexander V. Chernikov <melifaro@FreeBSD.org> > CommitDate: 2021-02-16 20:30:04 +0000 > > Fix dst/netmask handling in routing socket code. > > Traditionally routing socket code did almost zero checks on > the input message except for the most basic size checks. > > This resulted in the unclear KPI boundary for the routing system = > code > (`rtrequest*` and now `rib_action()`) w.r.t message validness. > > Multiple potential problems and nuances exists: > * Host bits in RTAX_DST sockaddr. Existing applications do send = > prefixes > with hostbits uncleared. Even `route(8)` does this, as they hope = > the kernel > would do the job of fixing it. Code inside `rib_action()` needs = > to handle > it on its own (see `rt_maskedcopy()` ugly hack). > * There are multiple way of adding the host route: it can be DST = > without > netmask or DST with /32(/128) netmask. Also, RTF_HOST has to be = > set correspondingly. > Currently, these 2 options create 2 DIFFERENT routes in the = > kernel. > * no sockaddr length/content checking for the "secondary" fields = > exists: nothing > stops rtsock application to send sockaddr_in with length of 25 = > (instead of 16). > Kernel will accept it, install to RIB as is and propagate to all = > rtsock consumers, > potentially triggering bugs in their code. Same goes for = > sin_port, sin_zero, etc. > > The goal of this change is to make rtsock verify all sockaddr and = > prefix consistency. > Said differently, `rib_action()` or internals should NOT require = > to change any of the > sockaddrs supplied by `rt_addrinfo` structure due to = > incorrectness. > > To be more specific, this change implements the following: > * sockaddr cleanup/validation check is added immediately after = > getting sockaddrs from rtm. > * Per-family dst/netmask checks clears host bits in dst and zeros = > all dst/netmask "secondary" fields. > * The same netmask checking code converts /32(/128) netmasks to = > "host" route case > (NULL netmask, RTF_HOST), removing the dualism. > * Instead of allowing ANY "known" sockaddr families (0<..<AF_MAX), = > allow only actually > supported ones (inet, inet6, link). > * Automatically convert `sockaddr_sdl` (AF_LINK) gateways to > `sockaddr_sdl_short`. > > Reported by: Guy Yur <guyyur at gmail.com> > Reviewed By: donner > Differential Revision: https://reviews.freebsd.org/D28668 > MFC after: 3 days > --- > sys/net/rtsock.c | 201 = > +++++++++++++++++++++++++++++++++- > tests/sys/net/routing/rtsock_common.h | 4 - > 2 files changed, 195 insertions(+), 10 deletions(-) > > +static int > +cleanup_xaddrs_inet(struct rt_addrinfo *info) > +{ > + struct sockaddr_in *dst_sa, *mask_sa; > + > + /* Check & fixup dst/netmask combination first */ > + dst_sa =3D (struct sockaddr_in *)info->rti_info[RTAX_DST]; > + mask_sa =3D (struct sockaddr_in *)info->rti_info[RTAX_NETMASK]; > + > + struct in_addr mask =3D { > + .s_addr =3D mask_sa ? mask_sa->sin_addr.s_addr : INADDR_BROADCAST, > + }; > + struct in_addr dst =3D { > + .s_addr =3D htonl(ntohl(dst_sa->sin_addr.s_addr) & = > ntohl(mask.s_addr)) > + }; > + This breaks things like `arp -d 10.0.2.1`. It always masks off the = network address, which is the right thing to do in the routing table, = but not in the arp table. I=E2=80=99ve worked around it for now with this hack: diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c index 3c1fea497af6..533076db99a5 100644 --- a/sys/net/rtsock.c +++ b/sys/net/rtsock.c @@ -638,9 +638,12 @@ fill_addrinfo(struct rt_msghdr *rtm, int len, = u_int fibnum, struct rt_addrinfo * return (EINVAL); info->rti_flags =3D rtm->rtm_flags; - error =3D cleanup_xaddrs(info); - if (error !=3D 0) - return (error); + /* XXX HACK */ + if (! (rtm->rtm_flags & RTF_LLDATA)) { + error =3D cleanup_xaddrs(info); + if (error !=3D 0) + return (error); + } saf =3D info->rti_info[RTAX_DST]->sa_family; /* * Verify that the caller has the appropriate privilege; = RTM_GET But I=E2=80=99m not totally happy with this, obviously. Best regards, Kristof From owner-dev-commits-src-main@freebsd.org Fri Feb 19 15:42:13 2021 Return-Path: <owner-dev-commits-src-main@freebsd.org> Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 70E645312F4; Fri, 19 Feb 2021 15:42:13 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Dhwn12qGRz4dgG; Fri, 19 Feb 2021 15:42:13 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 537976479; Fri, 19 Feb 2021 15:42:13 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 11JFgDnZ062367; Fri, 19 Feb 2021 15:42:13 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 11JFgDlp062366; Fri, 19 Feb 2021 15:42:13 GMT (envelope-from git) Date: Fri, 19 Feb 2021 15:42:13 GMT Message-Id: <202102191542.11JFgDlp062366@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Andrew Turner <andrew@FreeBSD.org> Subject: git: d765b211387c - main - Remove __XSCALE__ checks from the arm code MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: andrew X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: d765b211387c4c8a463086caeea8eb8836a50e57 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository <dev-commits-src-main.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/dev-commits-src-main>, <mailto:dev-commits-src-main-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/dev-commits-src-main/>; List-Post: <mailto:dev-commits-src-main@freebsd.org> List-Help: <mailto:dev-commits-src-main-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main>, <mailto:dev-commits-src-main-request@freebsd.org?subject=subscribe> X-List-Received-Date: Fri, 19 Feb 2021 15:42:13 -0000 The branch main has been updated by andrew: URL: https://cgit.FreeBSD.org/src/commit/?id=d765b211387c4c8a463086caeea8eb8836a50e57 commit d765b211387c4c8a463086caeea8eb8836a50e57 Author: Andrew Turner <andrew@FreeBSD.org> AuthorDate: 2021-02-19 15:22:13 +0000 Commit: Andrew Turner <andrew@FreeBSD.org> CommitDate: 2021-02-19 15:31:26 +0000 Remove __XSCALE__ checks from the arm code XScale support was removed over 2 years ago, remove the last __XSCALE__ checks from the arm MD code. Sponsored by: Innovate UK --- sys/arm/arm/dump_machdep.c | 3 --- sys/arm/arm/exception.S | 8 -------- 2 files changed, 11 deletions(-) diff --git a/sys/arm/arm/dump_machdep.c b/sys/arm/arm/dump_machdep.c index ead54ca7b225..c89a356d6228 100644 --- a/sys/arm/arm/dump_machdep.c +++ b/sys/arm/arm/dump_machdep.c @@ -63,9 +63,6 @@ dumpsys_wbinv_all(void) * part of stopping. */ dcache_wbinv_poc_all(); -#ifdef __XSCALE__ - xscale_cache_clean_minidata(); -#endif } void diff --git a/sys/arm/arm/exception.S b/sys/arm/arm/exception.S index 92e815b068fa..0416939cb199 100644 --- a/sys/arm/arm/exception.S +++ b/sys/arm/arm/exception.S @@ -236,10 +236,6 @@ END(exception_exit) * on exit (without transitioning back through the abort mode stack). */ ASENTRY_NP(prefetch_abort_entry) -#ifdef __XSCALE__ - nop /* Make absolutely sure any pending */ - nop /* imprecise aborts have occurred. */ -#endif sub lr, lr, #4 /* Adjust the lr. Transition to scv32 */ PUSHFRAMEINSVC /* mode stack, build trapframe there. */ adr lr, exception_exit /* Return from handler via standard */ @@ -256,10 +252,6 @@ END(prefetch_abort_entry) * on exit (without transitioning back through the abort mode stack). */ ASENTRY_NP(data_abort_entry) -#ifdef __XSCALE__ - nop /* Make absolutely sure any pending */ - nop /* imprecise aborts have occurred. */ -#endif sub lr, lr, #8 /* Adjust the lr. Transition to scv32 */ PUSHFRAMEINSVC /* mode stack, build trapframe there. */ adr lr, exception_exit /* Exception exit routine */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?26E2BA35-291E-4DC5-BDCB-D98347D7E24C>