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-...@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 },
        [IFLA_BR_FORWARD_DELAY] = { .type = NLA_U32 },
        [IFLA_BR_HELLO_TIME]    = { .type = NLA_U32 },
        [IFLA_BR_MAX_AGE]       = { .type = NLA_U32 },


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.

Thanks,
 Nik


Reply via email to