LGTM, thanks.
Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com>
On Mon, Oct 14, 2019 at 10:53 AM Yi-Hung Wei <yihung....@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.b...@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.b...@intel.com>
> Signed-off-by: David S. Miller <da...@davemloft.net>
>
> Signed-off-by: Yi-Hung Wei <yihung....@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
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
