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=2fe5a79425c79f7b828acd91da66d97230925fc8 > > 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 = (struct sockaddr_in *)info->rti_info[RTAX_DST]; > + mask_sa = (struct sockaddr_in *)info->rti_info[RTAX_NETMASK]; > + > + struct in_addr mask = { > + .s_addr = mask_sa ? mask_sa->sin_addr.s_addr : INADDR_BROADCAST, > + }; > + struct in_addr dst = { > + .s_addr = 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’ve 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 = rtm->rtm_flags; - error = cleanup_xaddrs(info); - if (error != 0) - return (error); + /* XXX HACK */ + if (! (rtm->rtm_flags & RTF_LLDATA)) { + error = cleanup_xaddrs(info); + if (error != 0) + return (error); + } saf = info->rti_info[RTAX_DST]->sa_family; /* * Verify that the caller has the appropriate privilege; RTM_GET But I’m 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×65b211387c4c8a463086caeea8eb8836a50e57 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>