netdev - Re: [ovs-dev] [PATCH] datapath: Fix for force/commit action failures

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]

Date:   Thu, 13 Jul 2017 17:08:38 +0000
From:   Darrell Ball <dball@...are.com>
To:     Greg Rose <gvrose8192@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "dev@...nvswitch.org" <dev@...nvswitch.org>
Subject: Re: [ovs-dev] [PATCH] datapath: Fix for force/commit action failures



On 7/13/17, 9:25 AM, "ovs-dev-bounces@...nvswitch.org on behalf of Greg Rose" <ovs-dev-bounces@...nvswitch.org on behalf of gvrose8192@...il.com> wrote:

    When there is an established connection in direction A->B, it is
    possible to receive a packet on port B which then executes
    ct(commit,force) without first performing ct() - ie, a lookup.
    In this case, we would expect that this packet can delete the existing
    entry so that we can commit a connection with direction B->A. However,
    currently we only perform a check in skb_nfct_cached() for whether
    OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie that a
    lookup previously occurred. In the above scenario, a lookup has not
    occurred but we should still be able to statelessly look up the
    existing entry and potentially delete the entry if it is in the
    opposite direction.
    
    This patch extends the check to also hint that if the action has the
    force flag set, then we will lookup the existing entry so that the
    force check at the end of skb_nfct_cached has the ability to delete
    the connection.
    
    CC: dev@...nvswitch.org
    CC: Pravin Shalar <pshelar@...ira.com>
    Signed-off-by: Joe Stringer <joe@....org>
    Signed-off-by: Greg Rose <gvrose8192@...il.com>
    ---
     net/openvswitch/conntrack.c | 12 ++++++++----
     1 file changed, 8 insertions(+), 4 deletions(-)
    
    diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
    index 08679eb..9041cf8 100644
    --- a/net/openvswitch/conntrack.c
    +++ b/net/openvswitch/conntrack.c
    @@ -641,17 +641,21 @@ static bool skb_nfct_cached(struct net *net,
     	ct = nf_ct_get(skb, &ctinfo);
     	/* If no ct, check if we have evidence that an existing conntrack entry
     	 * might be found for this skb.  This happens when we lose a skb->_nfct
    -	 * due to an upcall.  If the connection was not confirmed, it is not
    -	 * cached and needs to be run through conntrack again.
    +	 * due to an upcall, or if the direction is being forced.  If the
    +	 * connection was not confirmed, it is not cached and needs to be run
    +	 * through conntrack again.
     	 */
    -	if (!ct && key->ct_state & OVS_CS_F_TRACKED &&
    +	if ((!ct && (key->ct_state & OVS_CS_F_TRACKED) &&
     	    !(key->ct_state & OVS_CS_F_INVALID) &&
    -	    key->ct_zone == info->zone.id) {
    +	     key->ct_zone == info->zone.id) ||
    +	     (!key->ct_state && info->force)) {
     		ct = ovs_ct_find_existing(net, &info->zone, info->family, skb,
     					  !!(key->ct_state
     					     & OVS_CS_F_NAT_MASK));
     		if (ct)
     			nf_ct_get(skb, &ctinfo);
    +		else
    +			return false;

the above else case looks redundant since it maps to the following check
     	if (!ct)
     		return false;
which services a superset of the code flow.

     	}
     	if (!ct)
     		return false;
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@...nvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=5da6ykiSQJBoJXmpq6iVknmPAc5JDzlVngmp8j_xcXA&s=PsX-njQ_hFqy8P77KNEyX0i7u165p2Wrbg4uAYqfbGs&e=