[ovs-dev] [PATCH 06/11] datapath: genetlink: optionally validate strictly/dumps

Yifeng Sun pkusunyifeng at gmail.com
Mon Oct 14 23:35:46 UTC 2019


LGTM, thanks.

Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>

On Mon, Oct 14, 2019 at 10:53 AM Yi-Hung Wei <yihung.wei at gmail.com> wrote:
>
> This patch backports the following upstream commit within the
> openvswitch kernel module with some checks so that it also works
> in the older kernel.
>
> Upstream commit:
> commit ef6243acb4782df587a4d7d6c310fa5b5d82684b
> Author: Johannes Berg <johannes.berg at intel.com>
> Date:   Fri Apr 26 14:07:31 2019 +0200
>
>     genetlink: optionally validate strictly/dumps
>
>     Add options to strictly validate messages and dump messages,
>     sometimes perhaps validating dump messages non-strictly may
>     be required, so add an option for that as well.
>
>     Since none of this can really be applied to existing commands,
>     set the options everwhere using the following spatch:
>
>         @@
>         identifier ops;
>         expression X;
>         @@
>         struct genl_ops ops[] = {
>         ...,
>          {
>                 .cmd = X,
>         +       .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
>                 ...
>          },
>         ...
>         };
>
>     For new commands one should just not copy the .validate 'opt-out'
>     flags and thus get strict validation.
>
>     Signed-off-by: Johannes Berg <johannes.berg at intel.com>
>     Signed-off-by: David S. Miller <davem at davemloft.net>
>
> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
> ---
>  acinclude.m4         |  1 +
>  datapath/conntrack.c |  9 +++++++++
>  datapath/datapath.c  | 39 +++++++++++++++++++++++++++++++++++++++
>  datapath/meter.c     | 12 ++++++++++++
>  4 files changed, 61 insertions(+)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index fe121ab9126d..055f5387db19 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -817,6 +817,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>    OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], [genlmsg_parse])
>    OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], [genl_notify.*family],
>                    [OVS_DEFINE([HAVE_GENL_NOTIFY_TAKES_FAMILY])])
> +  OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], [genl_validate_flags])
>    OVS_FIND_PARAM_IFELSE([$KSRC/include/net/genetlink.h],
>                          [genl_notify], [net],
>                          [OVS_DEFINE([HAVE_GENL_NOTIFY_TAKES_NET])])
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index b11a30965147..0c0d43bec2e5 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -2283,18 +2283,27 @@ exit_err:
>
>  static struct genl_ops ct_limit_genl_ops[] = {
>         { .cmd = OVS_CT_LIMIT_CMD_SET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
>                                            * privilege. */
>                 .policy = ct_limit_policy,
>                 .doit = ovs_ct_limit_cmd_set,
>         },
>         { .cmd = OVS_CT_LIMIT_CMD_DEL,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
>                                            * privilege. */
>                 .policy = ct_limit_policy,
>                 .doit = ovs_ct_limit_cmd_del,
>         },
>         { .cmd = OVS_CT_LIMIT_CMD_GET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = 0,               /* OK for unprivileged users. */
>                 .policy = ct_limit_policy,
>                 .doit = ovs_ct_limit_cmd_get,
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 78e2e6310529..f4244ea09869 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -652,6 +652,9 @@ static const struct nla_policy packet_policy[OVS_PACKET_ATTR_MAX + 1] = {
>
>  static struct genl_ops dp_packet_genl_ops[] = {
>         { .cmd = OVS_PACKET_CMD_EXECUTE,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = packet_policy,
>           .doit = ovs_packet_cmd_execute
> @@ -1440,22 +1443,34 @@ static const struct nla_policy flow_policy[OVS_FLOW_ATTR_MAX + 1] = {
>
>  static struct genl_ops dp_flow_genl_ops[] = {
>         { .cmd = OVS_FLOW_CMD_NEW,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = flow_policy,
>           .doit = ovs_flow_cmd_new
>         },
>         { .cmd = OVS_FLOW_CMD_DEL,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = flow_policy,
>           .doit = ovs_flow_cmd_del
>         },
>         { .cmd = OVS_FLOW_CMD_GET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = 0,               /* OK for unprivileged users. */
>           .policy = flow_policy,
>           .doit = ovs_flow_cmd_get,
>           .dumpit = ovs_flow_cmd_dump
>         },
>         { .cmd = OVS_FLOW_CMD_SET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = flow_policy,
>           .doit = ovs_flow_cmd_set,
> @@ -1832,22 +1847,34 @@ static const struct nla_policy datapath_policy[OVS_DP_ATTR_MAX + 1] = {
>
>  static struct genl_ops dp_datapath_genl_ops[] = {
>         { .cmd = OVS_DP_CMD_NEW,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = datapath_policy,
>           .doit = ovs_dp_cmd_new
>         },
>         { .cmd = OVS_DP_CMD_DEL,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = datapath_policy,
>           .doit = ovs_dp_cmd_del
>         },
>         { .cmd = OVS_DP_CMD_GET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = 0,               /* OK for unprivileged users. */
>           .policy = datapath_policy,
>           .doit = ovs_dp_cmd_get,
>           .dumpit = ovs_dp_cmd_dump
>         },
>         { .cmd = OVS_DP_CMD_SET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = datapath_policy,
>           .doit = ovs_dp_cmd_set,
> @@ -2277,22 +2304,34 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
>
>  static struct genl_ops dp_vport_genl_ops[] = {
>         { .cmd = OVS_VPORT_CMD_NEW,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = vport_policy,
>           .doit = ovs_vport_cmd_new
>         },
>         { .cmd = OVS_VPORT_CMD_DEL,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = vport_policy,
>           .doit = ovs_vport_cmd_del
>         },
>         { .cmd = OVS_VPORT_CMD_GET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = 0,               /* OK for unprivileged users. */
>           .policy = vport_policy,
>           .doit = ovs_vport_cmd_get,
>           .dumpit = ovs_vport_cmd_dump
>         },
>         { .cmd = OVS_VPORT_CMD_SET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +         .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>           .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
>           .policy = vport_policy,
>           .doit = ovs_vport_cmd_set,
> diff --git a/datapath/meter.c b/datapath/meter.c
> index b0a92891c7c0..7d8f51a8fcd1 100644
> --- a/datapath/meter.c
> +++ b/datapath/meter.c
> @@ -538,11 +538,17 @@ bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
>
>  static struct genl_ops dp_meter_genl_ops[] = {
>         { .cmd = OVS_METER_CMD_FEATURES,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = 0,               /* OK for unprivileged users. */
>                 .policy = meter_policy,
>                 .doit = ovs_meter_cmd_features
>         },
>         { .cmd = OVS_METER_CMD_SET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
>                                            *  privilege.
>                                            */
> @@ -550,11 +556,17 @@ static struct genl_ops dp_meter_genl_ops[] = {
>                 .doit = ovs_meter_cmd_set,
>         },
>         { .cmd = OVS_METER_CMD_GET,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = 0,               /* OK for unprivileged users. */
>                 .policy = meter_policy,
>                 .doit = ovs_meter_cmd_get,
>         },
>         { .cmd = OVS_METER_CMD_DEL,
> +#ifdef HAVE_GENL_VALIDATE_FLAGS
> +               .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +#endif
>                 .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
>                                            *  privilege.
>                                            */
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list