From patchwork Tue Jun 19 03:16:26 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pablo Neira Ayuso X-Patchwork-Id: 165639 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3DF77B7011 for ; Tue, 19 Jun 2012 13:17:35 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752565Ab2FSDRN (ORCPT ); Mon, 18 Jun 2012 23:17:13 -0400 Received: from mail.us.es ([193.147.175.20]:53883 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752666Ab2FSDRM (ORCPT ); Mon, 18 Jun 2012 23:17:12 -0400 Received: (qmail 17455 invoked from network); 19 Jun 2012 05:17:11 +0200 Received: from unknown (HELO us.es) (192.168.2.12) by us.es with SMTP; 19 Jun 2012 05:17:11 +0200 Received: (qmail 4097 invoked by uid 507); 19 Jun 2012 03:17:10 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on antivirus2 X-Spam-Level: X-Spam-Status: No, score=-97.8 required=7.5 tests=BAYES_50,RCVD_IN_PBL, RCVD_IN_SORBS_DUL,RDNS_DYNAMIC,USER_IN_WHITELIST autolearn=disabled version=3.3.1 Received: from 127.0.0.1 by antivirus2 (envelope-from , uid 501) with qmail-scanner-2.08 (clamdscan: 0.97.4/15056. Clear:RC:1(127.0.0.1):. Processed in 0.024491 secs); 19 Jun 2012 03:17:10 -0000 Received: from unknown (HELO antivirus2) (127.0.0.1) by us.es with SMTP; 19 Jun 2012 03:17:10 -0000 Received: from 192.168.1.13 (192.168.1.13) by antivirus2 (F-Secure/fsigk_smtp/407/antivirus2); Tue, 19 Jun 2012 05:17:10 +0200 (CEST) X-Virus-Status: clean(F-Secure/fsigk_smtp/407/antivirus2) Received: (qmail 3210 invoked from network); 19 Jun 2012 05:18:31 +0200 Received: from 179.116.221.87.dynamic.jazztel.es (HELO localhost.localdomain) (pneira@us.es@87.221.116.179) by us.es with SMTP; 19 Jun 2012 05:18:31 +0200 From: pablo@netfilter.org To: netfilter-devel@vger.kernel.org Cc: davem@davemloft.net, netdev@vger.kernel.org Subject: [PATCH 1/4] netfilter: ctnetlink: fix NULL dereference while trying to change helper Date: Tue, 19 Jun 2012 05:16:26 +0200 Message-Id: <1340075789-6196-2-git-send-email-pablo@netfilter.org> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1340075789-6196-1-git-send-email-pablo@netfilter.org> References: <1340075789-6196-1-git-send-email-pablo@netfilter.org> Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org From: Pablo Neira Ayuso The patch 1afc56794e03: "netfilter: nf_ct_helper: implement variable length helper private data" from Jun 7, 2012, leads to the following Smatch complaint: net/netfilter/nf_conntrack_netlink.c:1231 ctnetlink_change_helper() error: we previously assumed 'help->helper' could be null (see line 1228) This NULL dereference can be triggered with the following sequence: 1) attach the helper for first time when the conntrack is created. 2) remove the helper module or detach the helper from the conntrack via ctnetlink. 3) attach helper again (the same or different one, no matter) to the that existing conntrack again via ctnetlink. This patch fixes the problem by removing the use case that allows you to re-assign again a helper for one conntrack entry via ctnetlink since I cannot find any practical use for it. Reported-by: Dan Carpenter Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_conntrack_netlink.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index ae156df..76271a1 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1224,19 +1224,12 @@ ctnetlink_change_helper(struct nf_conn *ct, const struct nlattr * const cda[]) if (helper->from_nlattr && helpinfo) helper->from_nlattr(helpinfo, ct); return 0; - } - if (help->helper) + } else return -EBUSY; - /* need to zero data of old helper */ - memset(help->data, 0, help->helper->data_len); - } else { - /* we cannot set a helper for an existing conntrack */ - return -EOPNOTSUPP; } - rcu_assign_pointer(help->helper, helper); - - return 0; + /* we cannot set a helper for an existing conntrack */ + return -EOPNOTSUPP; } static inline int