netdev - Re: [PATCH net-next 6/8] openvswitch: Allow update of dp with OVS_DP_CMD_NEW if NLM_F

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEP_g=8=dzm7NS5zx4-+wY+AXZLpym7JGQO9YTAgdTPm9v1mpQ@mail.gmail.com>
Date:	Mon, 25 Nov 2013 13:23:50 -0800
From:	Jesse Gross <jesse@...ira.com>
To:	Thomas Graf <tgraf@...g.ch>
Cc:	David Miller <davem@...emloft.net>,
	"dev@...nvswitch.org" <dev@...nvswitch.org>,
	netdev <netdev@...r.kernel.org>,
	Daniel Borkmann <dborkman@...hat.com>, ffusco@...hat.com,
	fleitner@...hat.com, Eric Dumazet <eric.dumazet@...il.com>,
	Ben Hutchings <bhutchings@...arflare.com>
Subject: Re: [PATCH net-next 6/8] openvswitch: Allow update of dp with
 OVS_DP_CMD_NEW if NLM_F_REPLACE is set

On Fri, Nov 22, 2013 at 8:56 AM, Thomas Graf <tgraf@...g.ch> wrote:
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 95d4424..3f1fb87 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c

I'm a little worried that this introduces some quirks to the interface. Such as:

> @@ -1190,6 +1185,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
>         struct vport *vport;
>         struct ovs_net *ovs_net;
>         int err, i;
> +       bool allocated = false;
>
>         err = -EINVAL;
>         if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])

This requires that DP_CMD_SET supply an OVS_DP_ATTR_UPCALL_PID
although I don't think that it's really necessary. In fact, we used to
completely ignore that field on SET since it's really only useful on
datapath creation and can otherwise be done more naturally through the
vport interface.

> @@ -1197,11 +1193,26 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
>
>         ovs_lock();
>
> +       dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
> +       if (!IS_ERR(dp)) {
> +               if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE)
> +                       goto update;

Conversely, this won't respect the UPCALL_PID field, which I would
expect it to since I think NLM_F_REPLACE should be roughly equivalent
to a delete and create. I believe that's the only field that is
missing although it seems easy to have the same problem as others are
added in the future.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html