Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 23 Jul 2020 12:19:30 -0700 (PDT)
Message-Id: <20200723.121930.163681559677190095.davem@davemloft.net>
To:     salyzyn@android.com
Cc:     linux-kernel@vger.kernel.org, kernel-team@android.com,
        netdev@vger.kernel.org, kuba@kernel.org, tgraf@suug.ch
Subject: Re: [PATCH] netlink: add buffer boundary checking
From:   David Miller <davem@davemloft.net>
In-Reply-To: <20200723182136.2550163-1-salyzyn@android.com>
References: <20200723182136.2550163-1-salyzyn@android.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: Mark Salyzyn <salyzyn@android.com>
Date: Thu, 23 Jul 2020 11:21:32 -0700

> Many of the nla_get_* inlines fail to check attribute's length before
> copying the content resulting in possible out-of-boundary accesses.
> Adjust the inlines to perform nla_len checking, for the most part
> using the nla_memcpy function to faciliate since these are not
> necessarily performance critical and do not need a likely fast path.
> 
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: kernel-team@android.com
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Thomas Graf <tgraf@suug.ch>
> Fixes: bfa83a9e03cf ("[NETLINK]: Type-safe netlink messages/attributes interface")

Please, let's avoid stuff like this.

Now it is going to be expensive to move several small attributes,
which is common.  And there's a multiplier when dumping, for example,
thousands of networking devices, routes, or whatever, and all of their
attributes in a dump.

If you can document actual out of bounds accesses, let's fix them.  Usually
contextually the attribute type and size has been validated by the time we
execute these accessors.

I'm not applying this, sorry.