mbox series

[net-next,v2,0/3] nexthop: More fine-grained policies for netlink message validation

Message ID cover.1611156111.git.petrm@nvidia.com
Headers show
Series nexthop: More fine-grained policies for netlink message validation | expand

Message

Petr Machata Jan. 20, 2021, 3:44 p.m. UTC
There is currently one policy that covers all attributes for next hop
object management. Actual validation is then done in code, which makes it
unobvious which attributes are acceptable when, and indeed that everything
is rejected as necessary.

In this series, split rtm_nh_policy to several policies that cover various
aspects of the next hop object configuration, and instead of open-coding
the validation, defer to nlmsg_parse(). This should make extending the next
hop code simpler as well, which will be relevant in near future for
resilient hashing implementation.

This was tested by running tools/testing/selftests/net/fib_nexthops.sh.
Additionally iproute2 was tweaked to issue "nexthop list id" as an
RTM_GETNEXTHOP dump request, instead of a straight get to test that
unexpected attributes are indeed rejected.

In patch #1, convert attribute validation in nh_valid_get_del_req().

In patch #2, convert nh_valid_dump_req().

In patch #3, rtm_nh_policy is cleaned up and renamed to rtm_nh_policy_new,
because after the above two patches, that is the only context that it is
used in.

v2:
- Patches #1, #2 and #3:
    - Do not specify size of the policy array. Use ARRAY_SIZE instead
      of NHA_MAX
- Patch #2:
    - Convert manual setting of true to nla_get_flag().

Petr Machata (3):
  nexthop: Use a dedicated policy for nh_valid_get_del_req()
  nexthop: Use a dedicated policy for nh_valid_dump_req()
  nexthop: Specialize rtm_nh_policy

 net/ipv4/nexthop.c | 105 +++++++++++++++++++--------------------------
 1 file changed, 43 insertions(+), 62 deletions(-)

Comments

David Ahern Jan. 21, 2021, 4:26 a.m. UTC | #1
On 1/20/21 8:44 AM, Petr Machata wrote:
> This function uses the global nexthop policy only to then bounce all

> arguments except for NHA_ID. Instead, just create a new policy that

> only includes the one allowed attribute.

> 

> Signed-off-by: Petr Machata <petrm@nvidia.com>

> ---

> 

> Notes:

>     v2:

>     - Do not specify size of the policy array. Use ARRAY_SIZE instead

>       of NHA_MAX

> 

>  net/ipv4/nexthop.c | 26 +++++++++-----------------

>  1 file changed, 9 insertions(+), 17 deletions(-)


Reviewed-by: David Ahern <dsahern@kernel.org>
David Ahern Jan. 21, 2021, 4:29 a.m. UTC | #2
On 1/20/21 8:44 AM, Petr Machata wrote:
> This policy is currently only used for creation of new next hops and new

> next hop groups. Rename it accordingly and remove the two attributes that

> are not valid in that context: NHA_GROUPS and NHA_MASTER.

> 

> For consistency with other policies, do not mention policy array size in

> the declarator, and replace NHA_MAX for ARRAY_SIZE as appropriate.

> 

> Note that with this commit, NHA_MAX and __NHA_MAX are not used anymore.

> Leave them in purely as a user API.

> 

> Signed-off-by: Petr Machata <petrm@nvidia.com>

> ---

> 

> Notes:

>     v2:

>     - Do not specify size of the policy array. Use ARRAY_SIZE instead

>       of NHA_MAX

> 

>  net/ipv4/nexthop.c | 23 +++++++++--------------

>  1 file changed, 9 insertions(+), 14 deletions(-)

> 


Reviewed-by: David Ahern <dsahern@kernel.org>
patchwork-bot+netdevbpf@kernel.org Jan. 21, 2021, 5:10 a.m. UTC | #3
Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Wed, 20 Jan 2021 16:44:09 +0100 you wrote:
> There is currently one policy that covers all attributes for next hop

> object management. Actual validation is then done in code, which makes it

> unobvious which attributes are acceptable when, and indeed that everything

> is rejected as necessary.

> 

> In this series, split rtm_nh_policy to several policies that cover various

> aspects of the next hop object configuration, and instead of open-coding

> the validation, defer to nlmsg_parse(). This should make extending the next

> hop code simpler as well, which will be relevant in near future for

> resilient hashing implementation.

> 

> [...]


Here is the summary with links:
  - [net-next,v2,1/3] nexthop: Use a dedicated policy for nh_valid_get_del_req()
    https://git.kernel.org/netdev/net-next/c/60f5ad5e19c0
  - [net-next,v2,2/3] nexthop: Use a dedicated policy for nh_valid_dump_req()
    https://git.kernel.org/netdev/net-next/c/44551bff290d
  - [net-next,v2,3/3] nexthop: Specialize rtm_nh_policy
    https://git.kernel.org/netdev/net-next/c/643d0878e674

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html