All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: davem@davemloft.net
Cc: Patrick McHardy <kaber@trash.net>, netfilter-devel@vger.kernel.org
Subject: [NETFILTER 03/03]: {nfnetlink,ip,ip6}_queue: fix skb_over_panic when enlarging packets
Date: Tue, 29 Apr 2008 00:06:44 +0200 (MEST)	[thread overview]
Message-ID: <20080428220647.18270.56131.sendpatchset@localhost.localdomain> (raw)
In-Reply-To: <20080428220643.18270.24909.sendpatchset@localhost.localdomain>

[NETFILTER]: {nfnetlink,ip,ip6}_queue: fix skb_over_panic when enlarging packets

While reinjecting *bigger* modified versions of IPv6 packets using
libnetfilter_queue, things work fine on a 2.6.24 kernel (2.6.22 too)
but I get the following on recents kernels (2.6.25, trace below is
against today's net-2.6 git tree):

skb_over_panic: text:c04fddb0 len:696 put:632 head:f7592c00 data:f7592c00 tail:0xf7592eb8 end:0xf7592e80 dev:eth0
------------[ cut here ]------------
invalid opcode: 0000 [#1] PREEMPT 
Process sendd (pid: 3657, ti=f6014000 task=f77c31d0 task.ti=f6014000)
Stack: c071e638 c04fddb0 000002b8 00000278 f7592c00 f7592c00 f7592eb8 f7592e80 
       f763c000 f6bc5200 f7592c40 f6015c34 c04cdbfc f6bc5200 00000278 f6015c60 
       c04fddb0 00000020 f72a10c0 f751b420 00000001 0000000a 000002b8 c065582c 
Call Trace:
 [<c04fddb0>] ? nfqnl_recv_verdict+0x1c0/0x2e0
 [<c04cdbfc>] ? skb_put+0x3c/0x40
 [<c04fddb0>] ? nfqnl_recv_verdict+0x1c0/0x2e0
 [<c04fd115>] ? nfnetlink_rcv_msg+0xf5/0x160
 [<c04fd03e>] ? nfnetlink_rcv_msg+0x1e/0x160
 [<c04fd020>] ? nfnetlink_rcv_msg+0x0/0x160
 [<c04f8ed7>] ? netlink_rcv_skb+0x77/0xa0
 [<c04fcefc>] ? nfnetlink_rcv+0x1c/0x30
 [<c04f8c73>] ? netlink_unicast+0x243/0x2b0
 [<c04cfaba>] ? memcpy_fromiovec+0x4a/0x70
 [<c04f9406>] ? netlink_sendmsg+0x1c6/0x270
 [<c04c8244>] ? sock_sendmsg+0xc4/0xf0
 [<c011970d>] ? set_next_entity+0x1d/0x50
 [<c0133a80>] ? autoremove_wake_function+0x0/0x40
 [<c0118f9e>] ? __wake_up_common+0x3e/0x70
 [<c0342fbf>] ? n_tty_receive_buf+0x34f/0x1280
 [<c011d308>] ? __wake_up+0x68/0x70
 [<c02cea47>] ? copy_from_user+0x37/0x70
 [<c04cfd7c>] ? verify_iovec+0x2c/0x90
 [<c04c837a>] ? sys_sendmsg+0x10a/0x230
 [<c011967a>] ? __dequeue_entity+0x2a/0xa0
 [<c011970d>] ? set_next_entity+0x1d/0x50
 [<c0345397>] ? pty_write+0x47/0x60
 [<c033d59b>] ? tty_default_put_char+0x1b/0x20
 [<c011d2e9>] ? __wake_up+0x49/0x70
 [<c033df99>] ? tty_ldisc_deref+0x39/0x90
 [<c033ff20>] ? tty_write+0x1a0/0x1b0
 [<c04c93af>] ? sys_socketcall+0x7f/0x260
 [<c0102ff9>] ? sysenter_past_esp+0x6a/0x91
 [<c05f0000>] ? snd_intel8x0m_probe+0x270/0x6e0
 =======================
Code: 00 00 89 5c 24 14 8b 98 9c 00 00 00 89 54 24 0c 89 5c 24 10 8b 40 50 89 4c 24 04 c7 04 24 38 e6 71 c0 89 44 24 08 e8 c4 46 c5 ff <0f> 0b eb fe 55 89 e5 56 89 d6 53 89 c3 83 ec 0c 8b 40 50 39 d0 
EIP: [<c04ccdfc>] skb_over_panic+0x5c/0x60 SS:ESP 0068:f6015bf8


Looking at the code, I ended up in nfq_mangle() function (called by
nfqnl_recv_verdict()) which performs a call to skb_copy_expand() due to
the increased size of data passed to the function. AFAICT, it should ask
for 'diff' instead of 'diff - skb_tailroom(e->skb)'. Because the
resulting sk_buff has not enough space to support the skb_put(skb, diff)
call a few lines later, this results in the call to skb_over_panic().

The patch below asks for allocation of a copy with enough space for
mangled packet and the same amount of headroom as old sk_buff. While
looking at how the regression appeared (e2b58a67), I noticed the same
pattern in ipq_mangle_ipv6() and ipq_mangle_ipv4(). The patch corrects
those locations too.

Tested with bigger reinjected IPv6 packets (nfqnl_mangle() path), things
are ok (2.6.25 and today's net-2.6 git tree).

Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit b8bd109c63a5d0d72530c7f59fc2412b03fa75ac
tree 4587cc797b38f3f133f7d198ecb6ffeadbaaaf97
parent 6317c53b124105c288604f51d50e2c759f6f333b
author Arnaud Ebalard <arno@natisbad.org> Tue, 29 Apr 2008 00:02:25 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 29 Apr 2008 00:02:25 +0200

 net/ipv4/netfilter/ip_queue.c   |    5 ++---
 net/ipv6/netfilter/ip6_queue.c  |    5 ++---
 net/netfilter/nfnetlink_queue.c |    5 ++---
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index 719be29..26a37ce 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -296,9 +296,8 @@ ipq_mangle_ipv4(ipq_verdict_msg_t *v, struct nf_queue_entry *e)
 		if (v->data_len > 0xFFFF)
 			return -EINVAL;
 		if (diff > skb_tailroom(e->skb)) {
-			nskb = skb_copy_expand(e->skb, 0,
-					       diff - skb_tailroom(e->skb),
-					       GFP_ATOMIC);
+			nskb = skb_copy_expand(e->skb, skb_headroom(e->skb),
+					       diff, GFP_ATOMIC);
 			if (!nskb) {
 				printk(KERN_WARNING "ip_queue: error "
 				      "in mangle, dropping packet\n");
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index 92a36c9..2eff3ae 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -298,9 +298,8 @@ ipq_mangle_ipv6(ipq_verdict_msg_t *v, struct nf_queue_entry *e)
 		if (v->data_len > 0xFFFF)
 			return -EINVAL;
 		if (diff > skb_tailroom(e->skb)) {
-			nskb = skb_copy_expand(e->skb, 0,
-					       diff - skb_tailroom(e->skb),
-					       GFP_ATOMIC);
+			nskb = skb_copy_expand(e->skb, skb_headroom(e->skb),
+					       diff, GFP_ATOMIC);
 			if (!nskb) {
 				printk(KERN_WARNING "ip6_queue: OOM "
 				      "in mangle, dropping packet\n");
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 2c9fe5c..3447025 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -454,9 +454,8 @@ nfqnl_mangle(void *data, int data_len, struct nf_queue_entry *e)
 		if (data_len > 0xFFFF)
 			return -EINVAL;
 		if (diff > skb_tailroom(e->skb)) {
-			nskb = skb_copy_expand(e->skb, 0,
-					       diff - skb_tailroom(e->skb),
-					       GFP_ATOMIC);
+			nskb = skb_copy_expand(e->skb, skb_headroom(e->skb),
+					       diff, GFP_ATOMIC);
 			if (!nskb) {
 				printk(KERN_WARNING "nf_queue: OOM "
 				      "in mangle, dropping packet\n");

  parent reply	other threads:[~2008-04-28 22:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-28 22:06 [NETFILTER 00/03]: Netfilter fixes Patrick McHardy
2008-04-28 22:06 ` [NETFILTER 01/03]: xt_TCPOPTSTRIP: signed tcphoff for ipv6_skip_exthdr() retval Patrick McHardy
2008-04-28 22:06 ` [NETFILTER 02/03]: x_tables: fix net namespace leak when reading /proc/net/xxx_tables_names Patrick McHardy
2008-04-28 22:06 ` Patrick McHardy [this message]
2008-04-29 10:16 ` [NETFILTER 00/03]: Netfilter fixes David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080428220647.18270.56131.sendpatchset@localhost.localdomain \
    --to=kaber@trash.net \
    --cc=davem@davemloft.net \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.