Smack: Correct use of netlbl_skbuff_err
diff mbox

Message ID c4aac746-357c-5f4f-bba0-cf2fda8848dc@schaufler-ca.com
State New
Headers show

Commit Message

Casey Schaufler July 21, 2016, 11:32 p.m. UTC
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

Comments

Paul Moore July 22, 2016, 2:58 p.m. UTC | #1
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.
Casey Schaufler July 22, 2016, 4:52 p.m. UTC | #2
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
Paul Moore July 22, 2016, 5:16 p.m. UTC | #3
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
Paul Moore July 22, 2016, 6:07 p.m. UTC | #4
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.
Casey Schaufler July 22, 2016, 6:19 p.m. UTC | #5
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

Patch
diff mbox

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);