From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Subject: [PATCH v3 2/3] Do error handling if __build_packet_message fails Date: Wed, 29 Oct 2014 10:04:53 -0200 Message-ID: References: <7be00bf663b212d4984414346da216cf17d3443d.1414584011.git.mleitner@redhat.com> To: netfilter-devel@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40877 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932648AbaJ2MFH (ORCPT ); Wed, 29 Oct 2014 08:05:07 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9TC579E018356 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 29 Oct 2014 08:05:07 -0400 Received: from localhost.localdomain.com (vpn1-7-3.gru2.redhat.com [10.97.7.3]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9TC54SS026188 for ; Wed, 29 Oct 2014 08:05:06 -0400 In-Reply-To: <7be00bf663b212d4984414346da216cf17d3443d.1414584011.git.mleitner@redhat.com> Sender: netfilter-devel-owner@vger.kernel.org List-ID: Currently, we don't check if __build_packet_message fails or not and that leaves it prone to sending incomplete messages to userspace. This commit fixes it by canceling the new message if there was any issue while building it and makes sure the skb is freed if it was the only messsage in it. Signed-off-by: Marcelo Ricardo Leitner --- net/netfilter/nfnetlink_log.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index b1e3a05794169283ed50d1c0fb4f44d9e7753eeb..05184a3d860885c10c3e977fdc47fb86e3528afa 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -568,10 +568,8 @@ __build_packet_message(struct nfnl_log_net *log, struct nlattr *nla; int size = nla_attr_size(data_len); - if (skb_tailroom(inst->skb) < nla_total_size(data_len)) { - printk(KERN_WARNING "nfnetlink_log: no tailroom!\n"); - return -1; - } + if (skb_tailroom(inst->skb) < nla_total_size(data_len)) + goto nla_put_failure; nla = (struct nlattr *)skb_put(inst->skb, nla_total_size(data_len)); nla->nla_type = NFULA_PAYLOAD; @@ -586,6 +584,7 @@ __build_packet_message(struct nfnl_log_net *log, nla_put_failure: PRINTR(KERN_ERR "nfnetlink_log: error creating log nlmsg\n"); + nlmsg_cancel(skb, nlh); return -1; } @@ -708,8 +707,9 @@ nfulnl_log_packet(struct net *net, inst->qlen++; - __build_packet_message(log, inst, skb, data_len, pf, - hooknum, in, out, prefix, plen); + if (__build_packet_message(log, inst, skb, data_len, pf, + hooknum, in, out, prefix, plen)) + goto build_failure; if (inst->qlen >= qthreshold) __nfulnl_flush(inst); @@ -726,6 +726,15 @@ unlock_and_release: instance_put(inst); return; +build_failure: + /* If no other messages in it, we're good to free it. */ + if (!inst->skb->len) { + kfree_skb(inst->skb); + inst->skb = NULL; + } + + inst->qlen--; + alloc_failure: /* FIXME: statistics */ goto unlock_and_release; -- 1.9.3