From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752149Ab0JLXi6 (ORCPT ); Tue, 12 Oct 2010 19:38:58 -0400 Received: from tundra.namei.org ([65.99.196.166]:45680 "EHLO tundra.namei.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751253Ab0JLXi4 (ORCPT ); Tue, 12 Oct 2010 19:38:56 -0400 Date: Wed, 13 Oct 2010 10:38:28 +1100 (EST) From: James Morris To: Eric Paris cc: linux-kernel@vger.kernel.org, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, paul.moore@hp.com, selinux@tycho.nsa.gov, sds@tycho.nsa.gov, jengelh@medozas.de, linux-security-module@vger.kernel.org, mr.dash.four@googlemail.com, pablo@netfilter.org Subject: Re: [PATCH 1/5] secmark: do not return early if there was no error In-Reply-To: <20101012154008.26943.44399.stgit@paris.rdu.redhat.com> Message-ID: References: <20101012154008.26943.44399.stgit@paris.rdu.redhat.com> User-Agent: Alpine 2.00 (LRH 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 12 Oct 2010, Eric Paris wrote: > Commit 4a5a5c73 attempted to pass decent error messages back to userspace for > netfilter errors. In xt_SECMARK.c however the patch screwed up and returned > on 0 (aka no error) early and didn't finish setting up secmark. This results > in a kernel BUG if you use SECMARK. > Does this need to go into current Linus? > ------------[ cut here ]------------ > kernel BUG at net/netfilter/xt_SECMARK.c:38! > invalid opcode: 0000 [#1] SMP > last sysfs file: /sys/devices/system/cpu/cpu2/cache/index2/shared_cpu_map > CPU 0 > Modules linked in: xt_SECMARK iptable_mangle nfs lockd fscache nfs_acl > auth_rpcgss sunrpc ip6t_REJECT nf_conntrack_ipv6 ip6table_filter ip6_tables > uinput virtio_net virtio_balloon i2c_piix4 i2c_core joydev microcode ipv6 > virtio_blk virtio_pci virtio_ring virtio [last unloaded: speedstep_lib] > > Pid: 0, comm: swapper Not tainted 2.6.36-0.8.rc2.git0.fc15.x86_64 #1 /KVM > RIP: 0010:[] [] secmark_tg+0x17/0x2e [xt_SECMARK] > RSP: 0018:ffff880003e03a40 EFLAGS: 00010202 > RAX: ffff88001f3074b0 RBX: ffff88001f3073f0 RCX: ffff88001f307490 > RDX: ffff88001f307401 RSI: ffff880003e03b30 RDI: ffff88001f18e500 > RBP: ffff880003e03a40 R08: 0000000000000002 R09: ffff880003e03a10 > R10: ffff880003fd2ad8 R11: ffffffff00000001 R12: ffff88001a85d498 > R13: ffffe8ffff808240 R14: ffff88001ac133ae R15: ffff88001f18e500 > FS: 0000000000000000(0000) GS:ffff880003e00000(0000) > knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > CR2: 000000000073b130 CR3: 000000000fdc0000 CR4: 00000000000006f0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > Process swapper (pid: 0, threadinfo ffffffff81a00000, task > ffffffff81a4b020) > Stack: > ffff880003e03b90 ffffffff814599ff 0000000000003a18 0000000000000000 > ffff880003e03b70 ffffffffffffffb8 0000000000000000 ffffffff82a39d60 > ffff880003e03a90 ffffffff8140db60 ffff880003e03ae0 ffffffff8140f2c0 > Call Trace: > > [] ipt_do_table+0x58a/0x6e2 > [] ? rcu_read_unlock+0x21/0x23 > [] ? nf_conntrack_find_get+0xb4/0xc7 > [] iptable_mangle_hook+0x10a/0x120 [iptable_mangle] > [] nf_iterate+0x46/0x89 > [] ? ip_rcv_finish+0x0/0x3c6 > [] nf_hook_slow+0x78/0xe3 > [] ? ip_rcv_finish+0x0/0x3c6 > [] ? run_filter+0x0/0xc0 > [] ? dev_seq_stop+0x8/0x10 > [] ? ip_rcv_finish+0x0/0x3c6 > [] NF_HOOK.clone.6+0x46/0x58 > [] ip_rcv+0x21f/0x24c > [] __netif_receive_skb+0x3e0/0x40a > [] netif_receive_skb+0x6c/0x73 > [] virtnet_poll+0x55b/0x6cb [virtio_net] > [] ? lock_release+0x19a/0x1a6 > [] net_rx_action+0xb1/0x1e3 > [] ? print_lock_contention_bug+0x1b/0xd5 > [] ? call_softirq+0x1c/0x30 > [] __do_softirq+0xfa/0x1cf > [] ? lock_release+0x19a/0x1a6 > [] call_softirq+0x1c/0x30 > [] do_softirq+0x4b/0xa2 > [] irq_exit+0x4a/0x8c > [] do_IRQ+0x9d/0xb4 > [] ret_from_intr+0x0/0x16 > > [] ? default_idle+0x3c/0x61 > [] ? native_safe_halt+0xb/0xd > [] ? trace_hardirqs_on+0xd/0xf > [] default_idle+0x41/0x61 > [] cpu_idle+0xb3/0x10f > [] rest_init+0xb7/0xbe > [] ? rest_init+0x0/0xbe > [] start_kernel+0x412/0x41d > [] x86_64_start_reservations+0xb1/0xb5 > [] x86_64_start_kernel+0xf8/0x107 > Code: 41 8a 04 24 88 05 1c 05 00 00 5a 89 d8 5b 41 5c 41 5d c9 c3 55 48 89 e5 > 0f 1f 44 00 00 48 8b 46 08 8a 10 3a 15 fd 04 00 00 74 02 <0f> 0b fe ca 75 0e > 8b 40 04 89 87 b4 00 00 00 83 c8 ff c9 c3 0f > RIP [] secmark_tg+0x17/0x2e [xt_SECMARK] > RSP > ---[ end trace 9aa5d06a71143e74 ]--- > > Signed-off-by: Eric Paris > --- > > net/netfilter/xt_SECMARK.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c > index 23b2d6c..364ad16 100644 > --- a/net/netfilter/xt_SECMARK.c > +++ b/net/netfilter/xt_SECMARK.c > @@ -101,7 +101,7 @@ static int secmark_tg_check(const struct xt_tgchk_param *par) > switch (info->mode) { > case SECMARK_MODE_SEL: > err = checkentry_selinux(info); > - if (err <= 0) > + if (err) > return err; > break; > > -- James Morris