From patchwork Fri Aug 11 18:16:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Desaulniers X-Patchwork-Id: 800714 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="EW+FqjxV"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3xTY9L6F3wz9sRg for ; Sat, 12 Aug 2017 04:16:22 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753584AbdHKSQP (ORCPT ); Fri, 11 Aug 2017 14:16:15 -0400 Received: from mail-pg0-f43.google.com ([74.125.83.43]:34413 "EHLO mail-pg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753271AbdHKSQN (ORCPT ); Fri, 11 Aug 2017 14:16:13 -0400 Received: by mail-pg0-f43.google.com with SMTP id u185so18451131pgb.1 for ; Fri, 11 Aug 2017 11:16:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=s5Osl9KTrLrIBoulWX0UqjNgW3s+d+vFKdRtR6Jmu08=; b=EW+FqjxVMXCE51wUqd2t/zva3/HOGgl9kDqBNMrUQBK19Bsja8Hww0/Zxv2+h2ASU4 M/yuO+zPJE5IqChgL2lLx/bOWw2Ea9egC+3ZRYq/CU788NIPWhPJFEYmu7idfPGSTusp 1GpRa9RhXGNAKzZeH3CFzWZoxHbLaNDIhW5cv640oAJ/1PVKDacWB5n+bH0RCu45GH1B K328dNQ2JDKtk05Icou3pZNVT69SGRCPdEhrJikHQN4PwTmdAIGuPG8iSKRBvUuv2ero d46dPPIjkk70GW9QcGboFGBJ9kDgFVsQkprNVDI2u4Xdzm7+XpF64PfNQYxXdYyCpu1Y yebA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=s5Osl9KTrLrIBoulWX0UqjNgW3s+d+vFKdRtR6Jmu08=; b=FQGE6H+iDM8MZWTG4mk0HQOYdZ7CY9itSGS4+rEJxm3JQQUoML5A8DP5gHAuwVhfqq hk0e9ZNIj/1fQKMr+IQAvKEW5XOV/zyTL5EtvOpLKw9IxCWK53CixawT5Z9Csv8eXxx/ tXCzKZMseb4+TBi3huV3XY2/QedH22Mmc/IOQLmgSUHwF5rk9BpLGaAZlBJg7IMqPhTR Ati0XiECJFJjgJH2XHdFhMEjlf9bP8WIWfCq1VQy1V6tTnb6rDu/Ca9ryE61WJdLDDiN Ui8QMgZL50QAv9WgnRvhdpPg5W3KL035bzpOp4soRarXkZBw9fJm3YLv4iHt2eAmFtMf 3VZQ== X-Gm-Message-State: AHYfb5it44v+a6uZn5W5REk3y5DlMJRMVNLj9t+DYkwn9Gx8T8x4pKhD 1oSDM93zA0x+D8gE X-Received: by 10.99.98.69 with SMTP id w66mr15908398pgb.58.1502475373141; Fri, 11 Aug 2017 11:16:13 -0700 (PDT) Received: from ndesaulniers0.mtv.corp.google.com ([100.98.120.50]) by smtp.gmail.com with ESMTPSA id k23sm4323010pfj.5.2017.08.11.11.16.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 11 Aug 2017 11:16:12 -0700 (PDT) From: Nick Desaulniers To: pablo@netfilter.org Cc: mka@chromium.org, lorenzo@google.com, Nick Desaulniers , Jozsef Kadlecsik , Florian Westphal , "David S. Miller" , Alexey Kuznetsov , Hideaki YOSHIFUJI , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] netfilter: nf_nat_h323: fix logical-not-parentheses warning Date: Fri, 11 Aug 2017 11:16:07 -0700 Message-Id: <20170811181607.33878-1-ndesaulniers@google.com> X-Mailer: git-send-email 2.14.0.434.g98096fd7a8-goog In-Reply-To: <20170811174259.GA3743@salvia> References: <20170811174259.GA3743@salvia> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Clang produces the following warning: net/ipv4/netfilter/nf_nat_h323.c:553:6: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses] if (!set_h225_addr(skb, protoff, data, dataoff, taddr, ^ add parentheses after the '!' to evaluate the comparison first add parentheses around left hand side expression to silence this warning There's not necessarily a bug here, but it's cleaner to return early, ex: if (x) return ... rather than: if (!x == 0) ... else return Also added a return code check that seemed to be missing in one instance. Signed-off-by: Nick Desaulniers --- Changes in v2: * Reorder if/else blocks to return early. * Also handle this for set_h245_addr(), not just set_h225_addr(). * Add return code check for the Gnomemeeting case. net/ipv4/netfilter/nf_nat_h323.c | 57 +++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c index 574f7ebba0b6..ac8342dcb55e 100644 --- a/net/ipv4/netfilter/nf_nat_h323.c +++ b/net/ipv4/netfilter/nf_nat_h323.c @@ -252,16 +252,16 @@ static int nat_rtp_rtcp(struct sk_buff *skb, struct nf_conn *ct, if (set_h245_addr(skb, protoff, data, dataoff, taddr, &ct->tuplehash[!dir].tuple.dst.u3, htons((port & htons(1)) ? nated_port + 1 : - nated_port)) == 0) { - /* Save ports */ - info->rtp_port[i][dir] = rtp_port; - info->rtp_port[i][!dir] = htons(nated_port); - } else { + nated_port))) { nf_ct_unexpect_related(rtp_exp); nf_ct_unexpect_related(rtcp_exp); return -1; } + /* Save ports */ + info->rtp_port[i][dir] = rtp_port; + info->rtp_port[i][!dir] = htons(nated_port); + /* Success */ pr_debug("nf_nat_h323: expect RTP %pI4:%hu->%pI4:%hu\n", &rtp_exp->tuple.src.u3.ip, @@ -370,15 +370,15 @@ static int nat_h245(struct sk_buff *skb, struct nf_conn *ct, /* Modify signal */ if (set_h225_addr(skb, protoff, data, dataoff, taddr, &ct->tuplehash[!dir].tuple.dst.u3, - htons(nated_port)) == 0) { - /* Save ports */ - info->sig_port[dir] = port; - info->sig_port[!dir] = htons(nated_port); - } else { + htons(nated_port))) { nf_ct_unexpect_related(exp); return -1; } + /* Save ports */ + info->sig_port[dir] = port; + info->sig_port[!dir] = htons(nated_port); + pr_debug("nf_nat_q931: expect H.245 %pI4:%hu->%pI4:%hu\n", &exp->tuple.src.u3.ip, ntohs(exp->tuple.src.u.tcp.port), @@ -462,24 +462,27 @@ static int nat_q931(struct sk_buff *skb, struct nf_conn *ct, /* Modify signal */ if (set_h225_addr(skb, protoff, data, 0, &taddr[idx], &ct->tuplehash[!dir].tuple.dst.u3, - htons(nated_port)) == 0) { - /* Save ports */ - info->sig_port[dir] = port; - info->sig_port[!dir] = htons(nated_port); - - /* Fix for Gnomemeeting */ - if (idx > 0 && - get_h225_addr(ct, *data, &taddr[0], &addr, &port) && - (ntohl(addr.ip) & 0xff000000) == 0x7f000000) { - set_h225_addr(skb, protoff, data, 0, &taddr[0], - &ct->tuplehash[!dir].tuple.dst.u3, - info->sig_port[!dir]); - } - } else { + htons(nated_port))) { nf_ct_unexpect_related(exp); return -1; } + /* Save ports */ + info->sig_port[dir] = port; + info->sig_port[!dir] = htons(nated_port); + + /* Fix for Gnomemeeting */ + if (idx > 0 && + get_h225_addr(ct, *data, &taddr[0], &addr, &port) && + (ntohl(addr.ip) & 0xff000000) == 0x7f000000) { + if (set_h225_addr(skb, protoff, data, 0, &taddr[0], + &ct->tuplehash[!dir].tuple.dst.u3, + info->sig_port[!dir])) { + nf_ct_unexpect_related(exp); + return -1; + } + } + /* Success */ pr_debug("nf_nat_ras: expect Q.931 %pI4:%hu->%pI4:%hu\n", &exp->tuple.src.u3.ip, @@ -550,9 +553,9 @@ static int nat_callforwarding(struct sk_buff *skb, struct nf_conn *ct, } /* Modify signal */ - if (!set_h225_addr(skb, protoff, data, dataoff, taddr, - &ct->tuplehash[!dir].tuple.dst.u3, - htons(nated_port)) == 0) { + if (set_h225_addr(skb, protoff, data, dataoff, taddr, + &ct->tuplehash[!dir].tuple.dst.u3, + htons(nated_port))) { nf_ct_unexpect_related(exp); return -1; }