[patch net-next] genetlink: add explicit ordering break check for split ops

[patch net-next] genetlink: add explicit ordering break check for split ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Currently, if cmd in the split ops array is of lower value than the
previous one, genl_validate_ops() continues to do the checks as if
the values are equal. This may result in non-obvious WARN_ON() hit in
these check.

Instead, check the incorrect ordering explicitly and put a WARN_ON()
in case it is broken.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/netlink/genetlink.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index a157247a1e45..6bd2ce51271f 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -593,8 +593,12 @@ static int genl_validate_ops(const struct genl_family *family)
 			return -EINVAL;
 
 		/* Check sort order */
-		if (a->cmd < b->cmd)
+		if (a->cmd < b->cmd) {
 			continue;
+		} else if (a->cmd > b->cmd) {
+			WARN_ON(1);
+			return -EINVAL;
+		}
 
 		if (a->internal_flags != b->internal_flags ||
 		    ((a->flags ^ b->flags) & ~(GENL_CMD_CAP_DO |
-- 
2.41.0

Re: [patch net-next] genetlink: add explicit ordering break check for split ops

[Jakub Kicinski]

On Thu, 20 Jul 2023 13:13:54 +0200 Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Currently, if cmd in the split ops array is of lower value than the
> previous one, genl_validate_ops() continues to do the checks as if
> the values are equal. This may result in non-obvious WARN_ON() hit in
> these check.
> 
> Instead, check the incorrect ordering explicitly and put a WARN_ON()
> in case it is broken.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

RE: [patch net-next] genetlink: add explicit ordering break check for split ops

[Keller, Jacob E]

> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, July 20, 2023 4:14 AM
> To: netdev@vger.kernel.org
> Cc: kuba@kernel.org; pabeni@redhat.com; davem@davemloft.net;
> edumazet@google.com; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [patch net-next] genetlink: add explicit ordering break check for split ops
> 
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Currently, if cmd in the split ops array is of lower value than the
> previous one, genl_validate_ops() continues to do the checks as if
> the values are equal. This may result in non-obvious WARN_ON() hit in
> these check.
> 
> Instead, check the incorrect ordering explicitly and put a WARN_ON()
> in case it is broken.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>


Good fix.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
>  net/netlink/genetlink.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> index a157247a1e45..6bd2ce51271f 100644
> --- a/net/netlink/genetlink.c
> +++ b/net/netlink/genetlink.c
> @@ -593,8 +593,12 @@ static int genl_validate_ops(const struct genl_family
> *family)
>  			return -EINVAL;
> 
>  		/* Check sort order */
> -		if (a->cmd < b->cmd)
> +		if (a->cmd < b->cmd) {
>  			continue;
> +		} else if (a->cmd > b->cmd) {
> +			WARN_ON(1);
> +			return -EINVAL;
> +		}
> 
>  		if (a->internal_flags != b->internal_flags ||
>  		    ((a->flags ^ b->flags) & ~(GENL_CMD_CAP_DO |
> --
> 2.41.0

Re: [patch net-next] genetlink: add explicit ordering break check for split ops

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 20 Jul 2023 13:13:54 +0200 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Currently, if cmd in the split ops array is of lower value than the
> previous one, genl_validate_ops() continues to do the checks as if
> the values are equal. This may result in non-obvious WARN_ON() hit in
> these check.
> 
> [...]

Here is the summary with links:
  - [net-next] genetlink: add explicit ordering break check for split ops
    https://git.kernel.org/netdev/net-next/c/5766946ea511

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