From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH v2 net-next 00/23] rtnetlink: Add support for rigid checking of data in dump request Date: Mon, 08 Oct 2018 10:40:29 -0700 (PDT) Message-ID: <20181008.104029.1379253630304646129.davem@davemloft.net> References: <20181008031644.15989-1-dsahern@kernel.org> <20181008110412.43o5qgaaqvsf2znw@brauner.io> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: dsahern@kernel.org, netdev@vger.kernel.org, jbenc@redhat.com, stephen@networkplumber.org, dsahern@gmail.com To: christian@brauner.io Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:57152 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726393AbeJIAxT (ORCPT ); Mon, 8 Oct 2018 20:53:19 -0400 In-Reply-To: <20181008110412.43o5qgaaqvsf2znw@brauner.io> Sender: netdev-owner@vger.kernel.org List-ID: From: Christian Brauner Date: Mon, 8 Oct 2018 13:04:13 +0200 > On Sun, Oct 07, 2018 at 08:16:21PM -0700, David Ahern wrote: >> From: David Ahern >> >> There are many use cases where a user wants to influence what is >> returned in a dump for some rtnetlink command: one is wanting data >> for a different namespace than the one the request is received and >> another is limiting the amount of data returned in the dump to a >> specific set of interest to userspace, reducing the cpu overhead of >> both kernel and userspace. Unfortunately, the kernel has historically >> not been strict with checking for the proper header or checking the >> values passed in the header. This lenient implementation has allowed >> iproute2 and other packages to pass any struct or data in the dump >> request as long as the family is the first byte. For example, ifinfomsg >> struct is used by iproute2 for all generic dump requests - links, >> addresses, routes and rules when it is really only valid for link >> requests. >> >> There is 1 is example where the kernel deals with the wrong struct: link >> dumps after VF support was added. Older iproute2 was sending rtgenmsg as >> the header instead of ifinfomsg so a patch was added to try and detect >> old userspace vs new: >> e5eca6d41f53 ("rtnetlink: fix userspace API breakage for iproute2 < v3.9.0") >> >> The latest example is Christian's patch set wanting to return addresses for >> a target namespace. It guesses the header struct is an ifaddrmsg and if it >> guesses wrong a netlink warning is generated in the kernel log on every >> address dump which is unacceptable. >> >> Another example where the kernel is a bit lenient is route dumps: iproute2 >> can send either a request with either ifinfomsg or a rtmsg as the header >> struct, yet the kernel always treats the header as an rtmsg (see >> inet_dump_fib and rtm_flags check). The header inconsistency impacts the >> ability to add kernel side filters for route dumps - a necessary feature >> for scale setups with 100k+ routes. >> >> How to resolve the problem of not breaking old userspace yet be able to >> move forward with new features such as kernel side filtering which are >> crucial for efficient operation at high scale? >> >> This patch set addresses the problem by adding a new socket flag, >> NETLINK_DUMP_STRICT_CHK, that userspace can use with setsockopt to >> request strict checking of headers and attributes on dump requests and >> hence unlock the ability to use kernel side filters as they are added. ... > At this point it's all nits so it's got my ACK but keener eyes than mine > might see other issues. > > Acked-by: Christian Brauner Series applied, thanks everyone. Please be on the lookout for userspace regressions from this patch set. Thanks.