| Message ID | c4aac746-357c-5f4f-bba0-cf2fda8848dc@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 21, 2016 at 7:32 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > Subject: [PATCH] Smack: Correct use of netlbl_skbuff_err > > Smack uses CIPSO to transmit the Smack label of the sending > process in most cases. A single label is designated as the > "ambient" label, and packets sent from this label go without > CIPSO headers. Until recently, netlbl_skbuff_err() seemed > happy to be used in either case, but mid 4-7 something changed. > This is the real fix, making Smack appropriately careful about > calling netlbl_skbuff_err() only when CIPSO is being used. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > security/smack/smack.h | 4 ++++ > security/smack/smack_lsm.c | 13 ++++++++----- > 2 files changed, 12 insertions(+), 5 deletions(-) I'm still very confused about the actual root cause of this problem because I don't understand how this patch really fixes anything. For example, looking at netlbl_skbuff_err(): void netlbl_skbuff_err(struct sk_buff *skb, int error, int gateway) { if (cipso_v4_optptr(skb)) cipso_v4_error(skb, error, gateway); } ... we see the first thing it does is check to see if a CIPSO option exists, and it is relatively straightforward: unsigned char *cipso_v4_optptr(const struct sk_buff *skb) { const struct iphdr *iph = ip_hdr(skb); unsigned char *optptr = (unsigned char *)&(ip_hdr(skb)[1]); int optlen; int taglen; for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) { if (optptr[0] == IPOPT_CIPSO) return optptr; taglen = optptr[1]; optlen -= taglen; optptr += taglen; } return NULL; } Further, the cipso_v4_error() function doesn't do anything crazy either: void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway) { if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES) return; if (gateway) icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0); else icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0); } I recognize that you are seeing a problem, I'm just skeptical that you've landed on a real solution. While your patch may resolve your current reproducer, I fear that this problem will rematerialize in the future.
On 7/22/2016 7:58 AM, Paul Moore wrote: > On Thu, Jul 21, 2016 at 7:32 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> Subject: [PATCH] Smack: Correct use of netlbl_skbuff_err >> >> Smack uses CIPSO to transmit the Smack label of the sending >> process in most cases. A single label is designated as the >> "ambient" label, and packets sent from this label go without >> CIPSO headers. Until recently, netlbl_skbuff_err() seemed >> happy to be used in either case, but mid 4-7 something changed. >> This is the real fix, making Smack appropriately careful about >> calling netlbl_skbuff_err() only when CIPSO is being used. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> security/smack/smack.h | 4 ++++ >> security/smack/smack_lsm.c | 13 ++++++++----- >> 2 files changed, 12 insertions(+), 5 deletions(-) > I'm still very confused about the actual root cause of this problem > because I don't understand how this patch really fixes anything. For > example, looking at netlbl_skbuff_err(): > > void netlbl_skbuff_err(struct sk_buff *skb, int error, int gateway) > { > if (cipso_v4_optptr(skb)) > cipso_v4_error(skb, error, gateway); > } > > ... we see the first thing it does is check to see if a CIPSO option > exists, and it is relatively straightforward: > > unsigned char *cipso_v4_optptr(const struct sk_buff *skb) > { > const struct iphdr *iph = ip_hdr(skb); > unsigned char *optptr = (unsigned char *)&(ip_hdr(skb)[1]); > int optlen; > int taglen; > > for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) { > if (optptr[0] == IPOPT_CIPSO) > return optptr; > taglen = optptr[1]; > optlen -= taglen; > optptr += taglen; > } > > return NULL; > } > > Further, the cipso_v4_error() function doesn't do anything crazy either: > > void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway) > { > if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES) > return; > > if (gateway) > icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0); > else > icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0); > } > > I recognize that you are seeing a problem, I'm just skeptical that > you've landed on a real solution. While your patch may resolve your > current reproducer, I fear that this problem will rematerialize in the > future. > Here's the gory detail of what happens before the patch. I am willing to accept that something is going on below that is responsible for the inappropriate frees. <5>[ 0.000000] Linux version 4.7.0-rc7 (cschaufler@localhost.localdomain) (gcc version 5.3.1 20160406 (Red Hat 5.3.1-6) (GCC) ) #108 SMP Wed Jul 20 17:54:24 PDT 2016 <6>[ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.7.0-rc7 root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root rd.lvm.lv=fedora/swap LANG=en_US.UTF-8 <snip> <7>[ 59.677158] ISO 9660 Extensions: Microsoft Joliet Level 3 <7>[ 59.703250] ISO 9660 Extensions: RRIP_1991A <6>[ 173.148630] TCP: request_sock_TCP: Possible SYN flooding on port 8000. Sending cookies. Check SNMP counters. <6>[ 173.158655] TCP: request_sock_TCP: Possible SYN flooding on port 8261. Sending cookies. Check SNMP counters. <6>[ 173.172573] TCP: request_sock_TCP: Possible SYN flooding on port 8265. Sending cookies. Check SNMP counters. <3>[ 173.174928] ============================================================================= <3>[ 173.174933] BUG kmalloc-64 (Not tainted): Redzone overwritten <3>[ 173.174935] ----------------------------------------------------------------------------- <3>[ 173.174935] <4>[ 173.174937] Disabling lock debugging due to kernel taint <3>[ 173.174939] INFO: 0xffff88007863efe8-0xffff88007863efef. First byte 0x6b instead of 0xcc <3>[ 173.174943] INFO: Allocated in 0x6b6b6b6b6b6b6b6b age=0 cpu=1 pid=2578 <3>[ 173.174944] 0x6b6b6b6b6b6b6b6b <3>[ 173.174946] 0x6b6b6b6b6b6b6b6b <3>[ 173.174947] 0x6b6b6b6b6b6b6b6b <3>[ 173.174949] 0xffffffff0000006b <3>[ 173.174953] ip_append_data.part.45+0xbd/0xe0 <3>[ 173.174955] ip_append_data+0x34/0x40 <3>[ 173.174958] icmp_push_reply+0x6a/0x120 <3>[ 173.174960] icmp_send+0x599/0x5f0 <3>[ 173.174964] cipso_v4_error+0x38/0x50 <3>[ 173.174967] netlbl_skbuff_err+0x2f/0x40 <3>[ 173.174971] smack_socket_sock_rcv_skb+0x1c4/0x240 <3>[ 173.174974] security_sock_rcv_skb+0x3b/0x50 <3>[ 173.174977] sk_filter+0x3f/0x280 <3>[ 173.174981] tcp_v4_rcv+0x98a/0xd80 <3>[ 173.174983] ip_local_deliver_finish+0xda/0x390 <3>[ 173.174984] ip_local_deliver+0x6f/0xf0 <3>[ 173.174987] INFO: Freed in smack_socket_sock_rcv_skb+0x106/0x240 age=0 cpu=3 pid=2556 <3>[ 173.174991] __slab_free+0x17f/0x2d0 <3>[ 173.174993] kfree+0x24f/0x2f0 <3>[ 173.174995] smack_socket_sock_rcv_skb+0x106/0x240 <3>[ 173.174997] security_sock_rcv_skb+0x3b/0x50 <3>[ 173.174999] sk_filter+0x3f/0x280 <3>[ 173.175001] tcp_v4_rcv+0x98a/0xd80 <3>[ 173.175003] ip_local_deliver_finish+0xda/0x390 <3>[ 173.175005] ip_local_deliver+0x6f/0xf0 <3>[ 173.175007] ip_rcv_finish+0x18f/0x620 <3>[ 173.175008] ip_rcv+0x28b/0x3c0 <3>[ 173.175011] __netif_receive_skb_core+0x625/0xbd0 <3>[ 173.175014] __netif_receive_skb+0x18/0x60 <3>[ 173.175015] process_backlog+0x8a/0x240 <3>[ 173.175017] net_rx_action+0x229/0x500 <3>[ 173.175021] __do_softirq+0xd7/0x577 <3>[ 173.175023] do_softirq_own_stack+0x1c/0x30 <3>[ 173.175025] INFO: Slab 0xffffea0001e18f80 objects=20 used=19 fp=0xffff88007863eaf8 flags=0x3fff8000004081 <3>[ 173.175027] INFO: Object 0xffff88007863efa8 @offset=4008 fp=0x6b6b6b6b6b6b6b6b <3>[ 173.175027] <3>[ 173.175030] Redzone ffff88007863efa0: cc cc cc cc cc cc cc cc ........ <3>[ 173.175032] Object ffff88007863efa8: 00 00 00 00 00 00 00 00 64 00 00 00 00 00 14 00 ........d....... <3>[ 173.175034] Object ffff88007863efb8: 6e 61 70 00 53 6e 61 70 00 53 6e 61 70 00 6b 6b nap.Snap.Snap.kk <3>[ 173.175036] Object ffff88007863efc8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk <3>[ 173.175037] Object ffff88007863efd8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk <3>[ 173.175039] Redzone ffff88007863efe8: 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkk <3>[ 173.175042] Padding ffff88007863f128: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ <4>[ 173.175045] CPU: 1 PID: 2578 Comm: smackpolyport Tainted: G B 4.7.0-rc7 #108 <4>[ 173.175047] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20150306_163512-brownie 04/01/2014 <4>[ 173.175049] 0000000000000086 000000009bacef8b ffff88007fc83690 ffffffff81445203 <4>[ 173.175054] ffff88007d007740 ffff88007863efa8 ffff88007fc836d0 ffffffff81272942 <4>[ 173.175059] 0000000000000008 ffff880000000001 ffff88007863eff0 ffff88007d007740 <4>[ 173.175064] Call Trace: <4>[ 173.175065] <IRQ> [<ffffffff81445203>] dump_stack+0x85/0xc2 <4>[ 173.175073] [<ffffffff81272942>] print_trailer+0x162/0x260 <4>[ 173.175076] [<ffffffff81272b05>] check_bytes_and_report+0xc5/0x110 <4>[ 173.175079] [<ffffffff8127371a>] check_object+0x1da/0x2a0 <4>[ 173.175083] [<ffffffff8127405b>] free_debug_processing+0x16b/0x3c0 <4>[ 173.175086] [<ffffffff817a0b61>] ? __ip_make_skb+0x351/0x400 <4>[ 173.175089] [<ffffffff812771bf>] __slab_free+0x17f/0x2d0 <4>[ 173.175093] [<ffffffff8111247f>] ? mark_held_locks+0x6f/0xa0 <4>[ 173.175096] [<ffffffff81277b4a>] ? kfree+0xda/0x2f0 <4>[ 173.175099] [<ffffffff817a0b61>] ? __ip_make_skb+0x351/0x400 <4>[ 173.175101] [<ffffffff817a0b61>] ? __ip_make_skb+0x351/0x400 <4>[ 173.175104] [<ffffffff81277cbf>] kfree+0x24f/0x2f0 <4>[ 173.175107] [<ffffffff817a0b61>] __ip_make_skb+0x351/0x400 <4>[ 173.175110] [<ffffffff817a0c70>] ip_push_pending_frames+0x20/0x40 <4>[ 173.175113] [<ffffffff817d687d>] icmp_push_reply+0xed/0x120 <4>[ 173.175116] [<ffffffff817d81b9>] icmp_send+0x599/0x5f0 <4>[ 173.175121] [<ffffffff818021a8>] cipso_v4_error+0x38/0x50 <4>[ 173.175124] [<ffffffff8187bf0f>] netlbl_skbuff_err+0x2f/0x40 <4>[ 173.175127] [<ffffffff813d58b4>] smack_socket_sock_rcv_skb+0x1c4/0x240 <4>[ 173.175130] [<ffffffff81112602>] ? trace_hardirqs_on_caller+0x152/0x200 <4>[ 173.175134] [<ffffffff810bb090>] ? __local_bh_enable_ip+0x70/0xc0 <4>[ 173.175138] [<ffffffff813cc56b>] security_sock_rcv_skb+0x3b/0x50 <4>[ 173.175141] [<ffffffff8176443f>] sk_filter+0x3f/0x280 <4>[ 173.175144] [<ffffffff817a40db>] ? __inet_lookup_established+0x6b/0x140 <4>[ 173.175147] [<ffffffff817c3eda>] tcp_v4_rcv+0x98a/0xd80 <4>[ 173.175151] [<ffffffff8179930a>] ip_local_deliver_finish+0xda/0x390 <4>[ 173.175153] [<ffffffff8179925f>] ? ip_local_deliver_finish+0x2f/0x390 <4>[ 173.175156] [<ffffffff8179980f>] ip_local_deliver+0x6f/0xf0 <4>[ 173.175159] [<ffffffff81799230>] ? ip_rcv_finish+0x620/0x620 <4>[ 173.175162] [<ffffffff81798d9f>] ip_rcv_finish+0x18f/0x620 <4>[ 173.175165] [<ffffffff81799b1b>] ip_rcv+0x28b/0x3c0 <4>[ 173.175167] [<ffffffff81798c10>] ? inet_del_offload+0x40/0x40 <4>[ 173.175171] [<ffffffff81747795>] __netif_receive_skb_core+0x625/0xbd0 <4>[ 173.175174] [<ffffffff8111247f>] ? mark_held_locks+0x6f/0xa0 <4>[ 173.175177] [<ffffffff81747d58>] __netif_receive_skb+0x18/0x60 <4>[ 173.175185] [<ffffffff81747e2a>] process_backlog+0x8a/0x240 <4>[ 173.175188] [<ffffffff81747e91>] ? process_backlog+0xf1/0x240 <4>[ 173.175191] [<ffffffff81749ef9>] net_rx_action+0x229/0x500 <4>[ 173.175193] [<ffffffff8111247f>] ? mark_held_locks+0x6f/0xa0 <4>[ 173.175196] [<ffffffff8189b867>] __do_softirq+0xd7/0x577 <4>[ 173.175200] [<ffffffff8179e42d>] ? ip_finish_output2+0x20d/0x640 <4>[ 173.175202] [<ffffffff8189a60c>] do_softirq_own_stack+0x1c/0x30 <4>[ 173.175204] <EOI> [<ffffffff810bb019>] do_softirq.part.15+0x59/0x60 <4>[ 173.175209] [<ffffffff810bb0d9>] __local_bh_enable_ip+0xb9/0xc0 <4>[ 173.175212] [<ffffffff8179e456>] ip_finish_output2+0x236/0x640 <4>[ 173.175215] [<ffffffff8179f3ee>] ? ip_finish_output+0x1ae/0x350 <4>[ 173.175218] [<ffffffff8179f3ee>] ip_finish_output+0x1ae/0x350 <4>[ 173.175222] [<ffffffff8178bef5>] ? nf_hook_slow+0x5/0x1a0 <4>[ 173.175224] [<ffffffff817a0223>] ip_output+0x83/0x140 <4>[ 173.175227] [<ffffffff8179f240>] ? ip_fragment.constprop.51+0x80/0x80 <4>[ 173.175230] [<ffffffff8179f6fd>] ip_local_out+0x3d/0x80 <4>[ 173.175233] [<ffffffff8179fb1d>] ip_queue_xmit+0x1dd/0x560 <4>[ 173.175236] [<ffffffff8179f945>] ? ip_queue_xmit+0x5/0x560 <4>[ 173.175239] [<ffffffff817ba03c>] tcp_transmit_skb+0x4ac/0x960 <4>[ 173.175241] [<ffffffff817304f7>] ? __alloc_skb+0x87/0x1e0 <4>[ 173.175245] [<ffffffff817ba6cc>] tcp_write_xmit+0x1dc/0x1140 <4>[ 173.175248] [<ffffffff817bb951>] __tcp_push_pending_frames+0x31/0xd0 <4>[ 173.175251] [<ffffffff817a819c>] tcp_push+0xec/0x110 <4>[ 173.175253] [<ffffffff817ac6ec>] tcp_sendmsg+0x43c/0xbd0 <4>[ 173.175256] [<ffffffff817df238>] inet_sendmsg+0xf8/0x1c0 <4>[ 173.175259] [<ffffffff817df145>] ? inet_sendmsg+0x5/0x1c0 <4>[ 173.175262] [<ffffffff81725c38>] sock_sendmsg+0x38/0x50 <4>[ 173.175265] [<ffffffff81726241>] SYSC_sendto+0x101/0x190 <4>[ 173.175269] [<ffffffff812a2a53>] ? vfs_read+0x123/0x140 <4>[ 173.175273] [<ffffffff8100301a>] ? trace_hardirqs_on_thunk+0x1a/0x1c <4>[ 173.175277] [<ffffffff8172717e>] SyS_sendto+0xe/0x10 <4>[ 173.175280] [<ffffffff818987bc>] entry_SYSCALL_64_fastpath+0x1f/0xbd <3>[ 173.175284] FIX kmalloc-64: Restoring 0xffff88007863efe8-0xffff88007863efef=0xcc <3>[ 173.175284] <3>[ 173.175304] FIX kmalloc-64: Object at 0xffff88007863efa8 not freed <3>[ 173.175317] ============================================================================= <3>[ 173.175320] BUG kmalloc-128 (Tainted: G B ): Redzone overwritten <3>[ 173.175321] ----------------------------------------------------------------------------- <3>[ 173.175321] <3>[ 173.175324] INFO: 0xffff880034501bb8-0xffff880034501bbf. First byte 0x6b instead of 0xcc <3>[ 173.175327] INFO: Allocated in 0x6b6b6b6b6b6b6b6b age=1 cpu=1 pid=2578 <3>[ 173.175329] 0x6b6b6b6b6b6b6b6b <3>[ 173.175331] 0x6b6b6b6b6b6b6b6b <3>[ 173.175333] 0xffffffff0000006b <3>[ 173.175335] icmp_send+0x101/0x5f0 <3>[ 173.175338] cipso_v4_error+0x38/0x50 <3>[ 173.175340] netlbl_skbuff_err+0x2f/0x40 <3>[ 173.175343] smack_socket_sock_rcv_skb+0x1c4/0x240 <3>[ 173.175345] security_sock_rcv_skb+0x3b/0x50 <3>[ 173.175348] sk_filter+0x3f/0x280 <3>[ 173.175351] tcp_v4_rcv+0x98a/0xd80 <3>[ 173.175353] ip_local_deliver_finish+0xda/0x390 <3>[ 173.175355] ip_local_deliver+0x6f/0xf0 <3>[ 173.175357] ip_rcv_finish+0x18f/0x620 <3>[ 173.175359] ip_rcv+0x28b/0x3c0 <3>[ 173.175362] __netif_receive_skb_core+0x625/0xbd0 <3>[ 173.175364] __netif_receive_skb+0x18/0x60 <3>[ 173.175369] INFO: Freed in rcu_nocb_kthread+0x6c5/0x1d90 age=22646 cpu=0 pid=9 <3>[ 173.175371] __slab_free+0x17f/0x2d0 <3>[ 173.175374] kfree+0x24f/0x2f0 <3>[ 173.175376] rcu_nocb_kthread+0x6c5/0x1d90 <3>[ 173.175380] kthread+0xf3/0x110 <3>[ 173.175382] ret_from_fork+0x1f/0x40 <3>[ 173.175384] INFO: Slab 0xffffea0000d14000 objects=17 used=14 fp=0xffff8800345015c8 flags=0x3fff8000004081 <3>[ 173.175386] INFO: Object 0xffff880034501b38 @offset=6968 fp=0x6b6b6b6b6b6b6b6b <3>[ 173.175386] <3>[ 173.175389] Redzone ffff880034501b30: cc cc cc cc cc cc cc cc ........ <3>[ 173.175392] Object ffff880034501b38: a8 84 4a 51 00 88 ff ff dc ff ff ff 62 00 00 00 ..JQ........b... <3>[ 173.175394] Object ffff880034501b48: 03 0a 00 00 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b ........kkkkkkkk <3>[ 173.175396] Object ffff880034501b58: 6b 6b 6b 6b 08 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b kkkk....kkkkkkkk <3>[ 173.175398] Object ffff880034501b68: 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 00 00 00 00 kkkkkkkk........ <3>[ 173.175399] Object ffff880034501b78: 64 00 00 00 00 00 14 00 6e 61 70 00 53 6e 61 70 d.......nap.Snap <3>[ 173.175401] Object ffff880034501b88: 00 53 6e 61 70 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b .Snap.kkkkkkkkkk <3>[ 173.175403] Object ffff880034501b98: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk <3>[ 173.175405] Object ffff880034501ba8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk <3>[ 173.175407] Redzone ffff880034501bb8: 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkk <3>[ 173.175409] Padding ffff880034501cf8: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ <4>[ 173.175412] CPU: 1 PID: 2578 Comm: smackpolyport Tainted: G B 4.7.0-rc7 #108 <4>[ 173.175414] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20150306_163512-brownie 04/01/2014 <4>[ 173.175415] 0000000000000086 000000009bacef8b ffff88007fc83740 ffffffff81445203 <4>[ 173.175420] ffff88007d007480 ffff880034501b38 ffff88007fc83780 ffffffff81272942 <4>[ 173.175424] 0000000000000008 ffff880000000001 ffff880034501bc0 ffff88007d007480 <4>[ 173.175429] Call Trace: <4>[ 173.175431] <IRQ> [<ffffffff81445203>] dump_stack+0x85/0xc2 <4>[ 173.175436] [<ffffffff81272942>] print_trailer+0x162/0x260 <4>[ 173.175440] [<ffffffff81272b05>] check_bytes_and_report+0xc5/0x110 <4>[ 173.175443] [<ffffffff8127371a>] check_object+0x1da/0x2a0 <4>[ 173.175446] [<ffffffff8127405b>] free_debug_processing+0x16b/0x3c0 <4>[ 173.175449] [<ffffffff817d7e2a>] ? icmp_send+0x20a/0x5f0 <4>[ 173.175452] [<ffffffff812771bf>] __slab_free+0x17f/0x2d0 <4>[ 173.175455] [<ffffffff8179f240>] ? ip_fragment.constprop.51+0x80/0x80 <4>[ 173.175458] [<ffffffff817a0c29>] ? ip_send_skb+0x19/0x40 <4>[ 173.175461] [<ffffffff817d7e2a>] ? icmp_send+0x20a/0x5f0 <4>[ 173.175464] [<ffffffff81277cbf>] kfree+0x24f/0x2f0 <4>[ 173.175466] [<ffffffff817d7e2a>] icmp_send+0x20a/0x5f0 <4>[ 173.175471] [<ffffffff818021a8>] cipso_v4_error+0x38/0x50 <4>[ 173.175473] [<ffffffff8187bf0f>] netlbl_skbuff_err+0x2f/0x40 <4>[ 173.175477] [<ffffffff813d58b4>] smack_socket_sock_rcv_skb+0x1c4/0x240 <4>[ 173.175480] [<ffffffff81112602>] ? trace_hardirqs_on_caller+0x152/0x200 <4>[ 173.175483] [<ffffffff810bb090>] ? __local_bh_enable_ip+0x70/0xc0 <4>[ 173.175487] [<ffffffff813cc56b>] security_sock_rcv_skb+0x3b/0x50 <4>[ 173.175490] [<ffffffff8176443f>] sk_filter+0x3f/0x280 <4>[ 173.175493] [<ffffffff817a40db>] ? __inet_lookup_established+0x6b/0x140 <4>[ 173.175496] [<ffffffff817c3eda>] tcp_v4_rcv+0x98a/0xd80 <4>[ 173.175499] [<ffffffff8179930a>] ip_local_deliver_finish+0xda/0x390 <4>[ 173.175502] [<ffffffff8179925f>] ? ip_local_deliver_finish+0x2f/0x390 <4>[ 173.175505] [<ffffffff8179980f>] ip_local_deliver+0x6f/0xf0 <4>[ 173.175508] [<ffffffff81799230>] ? ip_rcv_finish+0x620/0x620 <4>[ 173.175510] [<ffffffff81798d9f>] ip_rcv_finish+0x18f/0x620 <4>[ 173.175513] [<ffffffff81799b1b>] ip_rcv+0x28b/0x3c0 <4>[ 173.175516] [<ffffffff81798c10>] ? inet_del_offload+0x40/0x40 <4>[ 173.175519] [<ffffffff81747795>] __netif_receive_skb_core+0x625/0xbd0 <4>[ 173.175522] [<ffffffff8111247f>] ? mark_held_locks+0x6f/0xa0 <4>[ 173.175525] [<ffffffff81747d58>] __netif_receive_skb+0x18/0x60 <4>[ 173.175528] [<ffffffff81747e2a>] process_backlog+0x8a/0x240 <4>[ 173.175530] [<ffffffff81747e91>] ? process_backlog+0xf1/0x240 <4>[ 173.175533] [<ffffffff81749ef9>] net_rx_action+0x229/0x500 <4>[ 173.175536] [<ffffffff8111247f>] ? mark_held_locks+0x6f/0xa0 <4>[ 173.175540] [<ffffffff8189b867>] __do_softirq+0xd7/0x577 <4>[ 173.175543] [<ffffffff8179e42d>] ? ip_finish_output2+0x20d/0x640 <4>[ 173.175546] [<ffffffff8189a60c>] do_softirq_own_stack+0x1c/0x30 <4>[ 173.175548] <EOI> [<ffffffff810bb019>] do_softirq.part.15+0x59/0x60 <4>[ 173.175552] [<ffffffff810bb0d9>] __local_bh_enable_ip+0xb9/0xc0 <4>[ 173.175555] [<ffffffff8179e456>] ip_finish_output2+0x236/0x640 <4>[ 173.175558] [<ffffffff8179f3ee>] ? ip_finish_output+0x1ae/0x350 <4>[ 173.175561] [<ffffffff8179f3ee>] ip_finish_output+0x1ae/0x350 <4>[ 173.175564] [<ffffffff8178bef5>] ? nf_hook_slow+0x5/0x1a0 <4>[ 173.175567] [<ffffffff817a0223>] ip_output+0x83/0x140 <4>[ 173.175570] [<ffffffff8179f240>] ? ip_fragment.constprop.51+0x80/0x80 <4>[ 173.175573] [<ffffffff8179f6fd>] ip_local_out+0x3d/0x80 <4>[ 173.175576] [<ffffffff8179fb1d>] ip_queue_xmit+0x1dd/0x560 <4>[ 173.175579] [<ffffffff8179f945>] ? ip_queue_xmit+0x5/0x560 <4>[ 173.175582] [<ffffffff817ba03c>] tcp_transmit_skb+0x4ac/0x960 <4>[ 173.175584] [<ffffffff817304f7>] ? __alloc_skb+0x87/0x1e0 <4>[ 173.175587] [<ffffffff817ba6cc>] tcp_write_xmit+0x1dc/0x1140 <4>[ 173.175590] [<ffffffff817bb951>] __tcp_push_pending_frames+0x31/0xd0 <4>[ 173.175593] [<ffffffff817a819c>] tcp_push+0xec/0x110 <4>[ 173.175596] [<ffffffff817ac6ec>] tcp_sendmsg+0x43c/0xbd0 <4>[ 173.175599] [<ffffffff817df238>] inet_sendmsg+0xf8/0x1c0 <4>[ 173.175601] [<ffffffff817df145>] ? inet_sendmsg+0x5/0x1c0 <4>[ 173.175603] [<ffffffff81725c38>] sock_sendmsg+0x38/0x50 <4>[ 173.175606] [<ffffffff81726241>] SYSC_sendto+0x101/0x190 <4>[ 173.175609] [<ffffffff812a2a53>] ? vfs_read+0x123/0x140 <4>[ 173.175612] [<ffffffff8100301a>] ? trace_hardirqs_on_thunk+0x1a/0x1c <4>[ 173.175615] [<ffffffff8172717e>] SyS_sendto+0xe/0x10 <4>[ 173.175618] [<ffffffff818987bc>] entry_SYSCALL_64_fastpath+0x1f/0xbd <3>[ 173.175621] FIX kmalloc-128: Restoring 0xffff880034501bb8-0xffff880034501bbf=0xcc <3>[ 173.175621] <3>[ 173.175624] FIX kmalloc-128: Object at 0xffff880034501b38 not freed <6>[ 173.195241] TCP: request_sock_TCP: Possible SYN flooding on port 8266. Sending cookies. Check SNMP counters. <6>[ 173.218874] TCP: request_sock_TCP: Possible SYN flooding on port 8262. Sending cookies. Check SNMP counters. <6>[ 173.237509] TCP: request_sock_TCP: Possible SYN flooding on port 8267. Sending cookies. Check SNMP counters. <6>[ 173.280559] TCP: request_sock_TCP: Possible SYN flooding on port 8264. Sending cookies. Check SNMP counters. <6>[ 173.281284] TCP: request_sock_TCP: Possible SYN flooding on port 8269. Sending cookies. Check SNMP counters. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 22, 2016 at 12:52 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 7/22/2016 7:58 AM, Paul Moore wrote: >> On Thu, Jul 21, 2016 at 7:32 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >>> Subject: [PATCH] Smack: Correct use of netlbl_skbuff_err >>> >>> Smack uses CIPSO to transmit the Smack label of the sending >>> process in most cases. A single label is designated as the >>> "ambient" label, and packets sent from this label go without >>> CIPSO headers. Until recently, netlbl_skbuff_err() seemed >>> happy to be used in either case, but mid 4-7 something changed. >>> This is the real fix, making Smack appropriately careful about >>> calling netlbl_skbuff_err() only when CIPSO is being used. >>> >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>> --- >>> security/smack/smack.h | 4 ++++ >>> security/smack/smack_lsm.c | 13 ++++++++----- >>> 2 files changed, 12 insertions(+), 5 deletions(-) >> I'm still very confused about the actual root cause of this problem >> because I don't understand how this patch really fixes anything. For >> example, looking at netlbl_skbuff_err(): >> >> void netlbl_skbuff_err(struct sk_buff *skb, int error, int gateway) >> { >> if (cipso_v4_optptr(skb)) >> cipso_v4_error(skb, error, gateway); >> } >> >> ... we see the first thing it does is check to see if a CIPSO option >> exists, and it is relatively straightforward: >> >> unsigned char *cipso_v4_optptr(const struct sk_buff *skb) >> { >> const struct iphdr *iph = ip_hdr(skb); >> unsigned char *optptr = (unsigned char *)&(ip_hdr(skb)[1]); >> int optlen; >> int taglen; >> >> for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) { >> if (optptr[0] == IPOPT_CIPSO) >> return optptr; >> taglen = optptr[1]; >> optlen -= taglen; >> optptr += taglen; >> } >> >> return NULL; >> } >> >> Further, the cipso_v4_error() function doesn't do anything crazy either: >> >> void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway) >> { >> if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES) >> return; >> >> if (gateway) >> icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0); >> else >> icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0); >> } >> >> I recognize that you are seeing a problem, I'm just skeptical that >> you've landed on a real solution. While your patch may resolve your >> current reproducer, I fear that this problem will rematerialize in the >> future. > > Here's the gory detail of what happens before the patch. > I am willing to accept that something is going on below > that is responsible for the inappropriate frees. > > > <5>[ 0.000000] Linux version 4.7.0-rc7 (cschaufler@localhost.localdomain) (gcc version 5.3.1 20160406 (Red Hat 5.3.1-6) (GCC) ) #108 SMP Wed Jul 20 17:54:24 PDT 2016 > <6>[ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.7.0-rc7 root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root rd.lvm.lv=fedora/swap LANG=en_US.UTF-8 > > <snip> > > <7>[ 59.677158] ISO 9660 Extensions: Microsoft Joliet Level 3 > <7>[ 59.703250] ISO 9660 Extensions: RRIP_1991A > <6>[ 173.148630] TCP: request_sock_TCP: Possible SYN flooding on port 8000. Sending cookies. Check SNMP counters. > <6>[ 173.158655] TCP: request_sock_TCP: Possible SYN flooding on port 8261. Sending cookies. Check SNMP counters. > <6>[ 173.172573] TCP: request_sock_TCP: Possible SYN flooding on port 8265. Sending cookies. Check SNMP counters. > <3>[ 173.174928] ============================================================================= > <3>[ 173.174933] BUG kmalloc-64 (Not tainted): Redzone overwritten > <3>[ 173.174935] ----------------------------------------------------------------------------- > <3>[ 173.174935] > <4>[ 173.174937] Disabling lock debugging due to kernel taint > <3>[ 173.174939] INFO: 0xffff88007863efe8-0xffff88007863efef. First byte 0x6b instead of 0xcc > <3>[ 173.174943] INFO: Allocated in 0x6b6b6b6b6b6b6b6b age=0 cpu=1 pid=2578 > <3>[ 173.174944] 0x6b6b6b6b6b6b6b6b > <3>[ 173.174946] 0x6b6b6b6b6b6b6b6b > <3>[ 173.174947] 0x6b6b6b6b6b6b6b6b > <3>[ 173.174949] 0xffffffff0000006b > <3>[ 173.174953] ip_append_data.part.45+0xbd/0xe0 > <3>[ 173.174955] ip_append_data+0x34/0x40 > <3>[ 173.174958] icmp_push_reply+0x6a/0x120 > <3>[ 173.174960] icmp_send+0x599/0x5f0 > <3>[ 173.174964] cipso_v4_error+0x38/0x50 > <3>[ 173.174967] netlbl_skbuff_err+0x2f/0x40 > <3>[ 173.174971] smack_socket_sock_rcv_skb+0x1c4/0x240 It would be helpful to see the code at the addresses above, same with the freed memory below. Without your kernel binary I can only guess. > <3>[ 173.174974] security_sock_rcv_skb+0x3b/0x50 > <3>[ 173.174977] sk_filter+0x3f/0x280 > <3>[ 173.174981] tcp_v4_rcv+0x98a/0xd80 > <3>[ 173.174983] ip_local_deliver_finish+0xda/0x390 > <3>[ 173.174984] ip_local_deliver+0x6f/0xf0 > <3>[ 173.174987] INFO: Freed in smack_socket_sock_rcv_skb+0x106/0x240 age=0 cpu=3 pid=2556 > <3>[ 173.174991] __slab_free+0x17f/0x2d0 > <3>[ 173.174993] kfree+0x24f/0x2f0 > <3>[ 173.174995] smack_socket_sock_rcv_skb+0x106/0x240 > <3>[ 173.174997] security_sock_rcv_skb+0x3b/0x50 > <3>[ 173.174999] sk_filter+0x3f/0x280 > <3>[ 173.175001] tcp_v4_rcv+0x98a/0xd80 > <3>[ 173.175003] ip_local_deliver_finish+0xda/0x390 > <3>[ 173.175005] ip_local_deliver+0x6f/0xf0 > <3>[ 173.175007] ip_rcv_finish+0x18f/0x620 > <3>[ 173.175008] ip_rcv+0x28b/0x3c0 > <3>[ 173.175011] __netif_receive_skb_core+0x625/0xbd0 > <3>[ 173.175014] __netif_receive_skb+0x18/0x60 > <3>[ 173.175015] process_backlog+0x8a/0x240 > <3>[ 173.175017] net_rx_action+0x229/0x500 > <3>[ 173.175021] __do_softirq+0xd7/0x577 > <3>[ 173.175023] do_softirq_own_stack+0x1c/0x30
On Fri, Jul 22, 2016 at 1:53 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 7/22/2016 10:16 AM, Paul Moore wrote: >> It would be helpful to see the code at the addresses above, same with >> the freed memory below. Without your kernel binary I can only guess. >> > This is on linus 4.7-rc7 using the attached config file, running > on Fedora23. The problem can be reproduced using the tools in the > attached tar file. > > Extract the files in a safe directory. > > $ make smacktomux smackecho > $ su > ... > # ./testtcpsetxattr.sh > > I can send the binary if you'd prefer, although I doubt you'd want that. > > Thank you. I've got some travel coming up very shortly and the odds of me spinning up a new test VM to reproduce this before I leave is pretty slim.
On 7/22/2016 11:07 AM, Paul Moore wrote: > On Fri, Jul 22, 2016 at 1:53 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 7/22/2016 10:16 AM, Paul Moore wrote: >>> It would be helpful to see the code at the addresses above, same with >>> the freed memory below. Without your kernel binary I can only guess. >>> >> This is on linus 4.7-rc7 using the attached config file, running >> on Fedora23. The problem can be reproduced using the tools in the >> attached tar file. >> >> Extract the files in a safe directory. >> >> $ make smacktomux smackecho >> $ su >> ... >> # ./testtcpsetxattr.sh >> >> I can send the binary if you'd prefer, although I doubt you'd want that. >> >> Thank you. > I've got some travel coming up very shortly and the odds of me > spinning up a new test VM to reproduce this before I leave is pretty > slim. > OK, thanks. Any insights or suggestions would be very welcome. If you use qemu I might be able to get a disk image to you. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/smack/smack.h b/security/smack/smack.h index 6c91156..89e3ea9 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -97,7 +97,11 @@ struct socket_smack { struct smack_known *smk_out; /* outbound label */ struct smack_known *smk_in; /* inbound label */ struct smack_known *smk_packet; /* TCP peer label */ + int smk_cipso; }; +#define SMACK_SOCKET_UNSET 0 +#define SMACK_SOCKET_CIPSO 1 +#define SMACK_SOCKET_UNLABELED 2 /* * Inode smack data diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index e96080e..b4b1f97 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2312,6 +2312,7 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags) ssp->smk_in = skp; ssp->smk_out = skp; ssp->smk_packet = NULL; + ssp->smk_cipso = SMACK_SOCKET_UNSET; sk->sk_security = ssp; @@ -2460,11 +2461,13 @@ static int smack_netlabel(struct sock *sk, int labeled) bh_lock_sock_nested(sk); if (ssp->smk_out == smack_net_ambient || - labeled == SMACK_UNLABELED_SOCKET) + labeled == SMACK_UNLABELED_SOCKET) { netlbl_sock_delattr(sk); - else { + ssp->smk_cipso = SMACK_SOCKET_UNLABELED; + } else { skp = ssp->smk_out; rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel); + ssp->smk_cipso = SMACK_SOCKET_CIPSO; } bh_unlock_sock(sk); @@ -3969,7 +3972,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) netlbl_secattr_init(&secattr); rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr); - if (rc == 0) + if (rc == 0 && secattr.flags != NETLBL_SECATTR_NONE) skp = smack_from_secattr(&secattr, ssp); else skp = smack_net_ambient; @@ -3994,7 +3997,7 @@ access_check: rc = smk_access(skp, ssp->smk_in, MAY_WRITE, &ad); rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in, MAY_WRITE, rc); - if (rc != 0) + if (rc != 0 && ssp->smk_cipso == SMACK_SOCKET_CIPSO) netlbl_skbuff_err(skb, rc, 0); break; #if IS_ENABLED(CONFIG_IPV6) @@ -4249,7 +4252,7 @@ access_check: hskp = smack_ipv4host_label(&addr); rcu_read_unlock(); - if (hskp == NULL) + if (hskp == NULL && ssp->smk_cipso == SMACK_SOCKET_CIPSO) rc = netlbl_req_setattr(req, &skp->smk_netlabel); else netlbl_req_delattr(req);
Subject: [PATCH] Smack: Correct use of netlbl_skbuff_err Smack uses CIPSO to transmit the Smack label of the sending process in most cases. A single label is designated as the "ambient" label, and packets sent from this label go without CIPSO headers. Until recently, netlbl_skbuff_err() seemed happy to be used in either case, but mid 4-7 something changed. This is the real fix, making Smack appropriately careful about calling netlbl_skbuff_err() only when CIPSO is being used. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- security/smack/smack.h | 4 ++++ security/smack/smack_lsm.c | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html