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

["Keller, Jacob E" <jacob.e.keller@intel.com>]

> -----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