On Thu, Sep 21, 2023 at 01:14:43PM +0300, Nikolay Aleksandrov wrote:
On 9/21/23 10:23, Johannes Nixdorf wrote:
On Wed, Sep 20, 2023 at 01:46:02PM +0300, Nikolay Aleksandrov wrote:
On 9/19/23 11:12, Johannes Nixdorf wrote:
Set any new attributes added to br_policy to be parsed strictly, to prevent userspace from passing garbage.
Signed-off-by: Johannes Nixdorf jnixdorf-oss@avm.de
net/bridge/br_netlink.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 10f0d33d8ccf..505683ef9a26 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -1229,6 +1229,8 @@ static size_t br_port_get_slave_size(const struct net_device *brdev, } static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = {
- [IFLA_BR_UNSPEC] = { .strict_start_type =
IFLA_BR_MCAST_QUERIER_STATE + 1 },instead of IFLA_BR_MCAST_QUERIER_STATE + 1, why not move around the patch and just use the new attribute name? These are uapi, they won't change.
I wanted to avoid having a state between the two commits where the new attributes are already added, but not yet strictly verified. Otherwise they would present a slightly different UAPI at that one commit boundary than after this commit.
That's not really a problem, the attribute is the same.
This is also not the only place in the kernel where strict_start_type is specified that way. See e.g. commit c00041cf1cb8 ("net: bridge: Set strict_start_type at two policies"), even though that seems mostly be done to turn on strict_start_type preemtively, not in the same series that adds the new attribute.
Please, just use the new attribute to be more explicit where the strict parsing starts.
Ok. I've changed it locally for v5.