* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-29 10:26 James Morris
2017-11-29 12:29 ` Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 38+ messages in thread
From: James Morris @ 2017-11-29 10:26 UTC (permalink / raw)
To: Paul Moore, Eric Dumazet; +Cc: selinux, netdev, Stephen Smalley
I'm seeing a kernel stack corruption bug (detected via gcc) when running
the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket test:
https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests/inet_socket/test
# Verify that unauthorized client cannot communicate with the server.
$result = system
"runcon -t test_inet_bad_client_t -- $basedir/client stream 127.0.0.1 65535 2>&1";
This correctlly causes an access control error in the Netlabel code, and
the bug seems to be triggered during the ICMP send:
[ 339.806024] SELinux: failure in selinux_parse_skb(), unable to parse packet
[ 339.822505] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: ffffffff81745af5
[ 339.822505]
[ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-test #15
[ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS FWKT68A 01/19/2017
[ 339.885060] Call Trace:
[ 339.896875] <IRQ>
[ 339.908103] dump_stack+0x63/0x87
[ 339.920645] panic+0xe8/0x248
[ 339.932668] ? ip_push_pending_frames+0x33/0x40
[ 339.946328] ? icmp_send+0x525/0x530
[ 339.958861] ? kfree_skbmem+0x60/0x70
[ 339.971431] __stack_chk_fail+0x1b/0x20
[ 339.984049] icmp_send+0x525/0x530
[ 339.996205] ? netlbl_skbuff_err+0x36/0x40
[ 340.008997] ? selinux_netlbl_err+0x11/0x20
[ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
[ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
[ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
[ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
[ 340.074562] ? tcp_filter+0x2c/0x40
[ 340.086400] ? tcp_v4_rcv+0x820/0xa20
[ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
[ 340.111279] ? ip_local_deliver+0x6f/0xe0
[ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
[ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
[ 340.147442] ? ip_rcv+0x27c/0x3c0
[ 340.158668] ? inet_del_offload+0x40/0x40
[ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
[ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
[ 340.195282] ? __netif_receive_skb+0x18/0x60
[ 340.207288] ? process_backlog+0x95/0x140
[ 340.218948] ? net_rx_action+0x26c/0x3b0
[ 340.230416] ? __do_softirq+0xc9/0x26a
[ 340.241625] ? do_softirq_own_stack+0x2a/0x40
[ 340.253368] </IRQ>
[ 340.262673] ? do_softirq+0x50/0x60
[ 340.273450] ? __local_bh_enable_ip+0x57/0x60
[ 340.285045] ? ip_finish_output2+0x175/0x350
[ 340.296403] ? ip_finish_output+0x127/0x1d0
[ 340.307665] ? nf_hook_slow+0x3c/0xb0
[ 340.318230] ? ip_output+0x72/0xe0
[ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
[ 340.340070] ? ip_local_out+0x35/0x40
[ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
[ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
[ 340.372484] ? __skb_clone+0x2e/0x130
[ 340.382633] ? tcp_transmit_skb+0x558/0xa10
[ 340.393262] ? tcp_connect+0x938/0xad0
[ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
[ 340.414206] ? tcp_v4_connect+0x457/0x4e0
[ 340.424471] ? __inet_stream_connect+0xb3/0x300
[ 340.435195] ? inet_stream_connect+0x3b/0x60
[ 340.445607] ? SYSC_connect+0xd9/0x110
[ 340.455455] ? __audit_syscall_entry+0xaf/0x100
[ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
[ 340.476636] ? __audit_syscall_exit+0x209/0x290
[ 340.487151] ? SyS_connect+0xe/0x10
[ 340.496453] ? do_syscall_64+0x67/0x1b0
[ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
[ 340.516693] Kernel Offset: disabled
[ 340.526393] Rebooting in 11 seconds..
This is mostly reliable, and I'm only seeing it on bare metal (not in a
virtualbox vm).
The SELinux skb parse error at the start only sometimes appears, and
looking at the code, I suspect some kind of memory corruption being the
cause at that point (basic packet header checks).
I bisected the bug down to the following change:
commit bffa72cf7f9df842f0016ba03586039296b4caaf
Author: Eric Dumazet <edumazet@google.com>
Date: Tue Sep 19 05:14:24 2017 -0700
net: sk_buff rbnode reorg
...
Anyone else able to reproduce this, or have any ideas on what's happening?
- James
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 10:26 [BUG] kernel stack corruption during/after Netlabel error James Morris
@ 2017-11-29 12:29 ` Eric Dumazet
2017-11-29 17:31 ` Stephen Smalley
2017-11-30 0:22 ` Casey Schaufler
2 siblings, 0 replies; 38+ messages in thread
From: Eric Dumazet @ 2017-11-29 12:29 UTC (permalink / raw)
To: James Morris; +Cc: Paul Moore, selinux, netdev, Stephen Smalley
On Wed, Nov 29, 2017 at 2:26 AM, James Morris <james.l.morris@oracle.com> wrote:
> I'm seeing a kernel stack corruption bug (detected via gcc) when running
> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket test:
>
> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests/inet_socket/test
>
> # Verify that unauthorized client cannot communicate with the server.
> $result = system
> "runcon -t test_inet_bad_client_t -- $basedir/client stream 127.0.0.1 65535 2>&1";
>
> This correctlly causes an access control error in the Netlabel code, and
> the bug seems to be triggered during the ICMP send:
>
> [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to parse packet
> [ 339.822505] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: ffffffff81745af5
> [ 339.822505]
> [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-test #15
> [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS FWKT68A 01/19/2017
> [ 339.885060] Call Trace:
> [ 339.896875] <IRQ>
> [ 339.908103] dump_stack+0x63/0x87
> [ 339.920645] panic+0xe8/0x248
> [ 339.932668] ? ip_push_pending_frames+0x33/0x40
> [ 339.946328] ? icmp_send+0x525/0x530
> [ 339.958861] ? kfree_skbmem+0x60/0x70
> [ 339.971431] __stack_chk_fail+0x1b/0x20
> [ 339.984049] icmp_send+0x525/0x530
> [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
> [ 340.008997] ? selinux_netlbl_err+0x11/0x20
> [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
> [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
> [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
> [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
> [ 340.074562] ? tcp_filter+0x2c/0x40
> [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
> [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
> [ 340.111279] ? ip_local_deliver+0x6f/0xe0
> [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
> [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
> [ 340.147442] ? ip_rcv+0x27c/0x3c0
> [ 340.158668] ? inet_del_offload+0x40/0x40
> [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
> [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
> [ 340.195282] ? __netif_receive_skb+0x18/0x60
> [ 340.207288] ? process_backlog+0x95/0x140
> [ 340.218948] ? net_rx_action+0x26c/0x3b0
> [ 340.230416] ? __do_softirq+0xc9/0x26a
> [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
> [ 340.253368] </IRQ>
> [ 340.262673] ? do_softirq+0x50/0x60
> [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
> [ 340.285045] ? ip_finish_output2+0x175/0x350
> [ 340.296403] ? ip_finish_output+0x127/0x1d0
> [ 340.307665] ? nf_hook_slow+0x3c/0xb0
> [ 340.318230] ? ip_output+0x72/0xe0
> [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
> [ 340.340070] ? ip_local_out+0x35/0x40
> [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
> [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
> [ 340.372484] ? __skb_clone+0x2e/0x130
> [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
> [ 340.393262] ? tcp_connect+0x938/0xad0
> [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
> [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
> [ 340.424471] ? __inet_stream_connect+0xb3/0x300
> [ 340.435195] ? inet_stream_connect+0x3b/0x60
> [ 340.445607] ? SYSC_connect+0xd9/0x110
> [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
> [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
> [ 340.476636] ? __audit_syscall_exit+0x209/0x290
> [ 340.487151] ? SyS_connect+0xe/0x10
> [ 340.496453] ? do_syscall_64+0x67/0x1b0
> [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
> [ 340.516693] Kernel Offset: disabled
> [ 340.526393] Rebooting in 11 seconds..
>
> This is mostly reliable, and I'm only seeing it on bare metal (not in a
> virtualbox vm).
>
> The SELinux skb parse error at the start only sometimes appears, and
> looking at the code, I suspect some kind of memory corruption being the
> cause at that point (basic packet header checks).
>
> I bisected the bug down to the following change:
>
> commit bffa72cf7f9df842f0016ba03586039296b4caaf
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Sep 19 05:14:24 2017 -0700
>
> net: sk_buff rbnode reorg
> ...
>
>
> Anyone else able to reproduce this, or have any ideas on what's happening?
>
>
Hi James, thanks for the report.
Issue here is that icmp_send() used to be called with skb_in->dev ==
NULL or a valid device pointer ?
After my patch, skb_in->dev is aliased with part of skb_in->rbnode
(rb_left pointer)
So this code in icmp_send() might be fooled :
if (!(skb_in->dev && (skb_in->dev->flags&IFF_LOOPBACK)) &&
!icmpv4_global_allow(net, type, code))
goto out_bh_enable;
Although TCP stack should not manipulate skb->rbnode before the calls
to tcp_filter() (and thus security_sock_rcv_skb())
So at the point security_sock_rcv_skb is called, skb->dev should still be valid.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 10:26 [BUG] kernel stack corruption during/after Netlabel error James Morris
2017-11-29 12:29 ` Eric Dumazet
@ 2017-11-29 17:31 ` Stephen Smalley
2017-11-29 17:34 ` Eric Dumazet
2017-11-30 0:22 ` Casey Schaufler
2 siblings, 1 reply; 38+ messages in thread
From: Stephen Smalley @ 2017-11-29 17:31 UTC (permalink / raw)
To: James Morris, Paul Moore, Eric Dumazet; +Cc: netdev, selinux
On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
> I'm seeing a kernel stack corruption bug (detected via gcc) when
> running
> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket
> test:
>
> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests
> /inet_socket/test
>
> # Verify that unauthorized client cannot communicate with the
> server.
> $result = system
> "runcon -t test_inet_bad_client_t -- $basedir/client stream
> 127.0.0.1 65535 2>&1";
>
> This correctlly causes an access control error in the Netlabel code,
> and
> the bug seems to be triggered during the ICMP send:
>
> [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
> parse packet
> [ 339.822505] Kernel panic - not syncing: stack-protector: Kernel
> stack is corrupted in: ffffffff81745af5
> [ 339.822505]
> [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-
> test #15
> [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
> FWKT68A 01/19/2017
> [ 339.885060] Call Trace:
> [ 339.896875] <IRQ>
> [ 339.908103] dump_stack+0x63/0x87
> [ 339.920645] panic+0xe8/0x248
> [ 339.932668] ? ip_push_pending_frames+0x33/0x40
> [ 339.946328] ? icmp_send+0x525/0x530
> [ 339.958861] ? kfree_skbmem+0x60/0x70
> [ 339.971431] __stack_chk_fail+0x1b/0x20
> [ 339.984049] icmp_send+0x525/0x530
> [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
> [ 340.008997] ? selinux_netlbl_err+0x11/0x20
> [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
> [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
> [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
> [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
> [ 340.074562] ? tcp_filter+0x2c/0x40
> [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
> [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
> [ 340.111279] ? ip_local_deliver+0x6f/0xe0
> [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
> [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
> [ 340.147442] ? ip_rcv+0x27c/0x3c0
> [ 340.158668] ? inet_del_offload+0x40/0x40
> [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
> [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
> [ 340.195282] ? __netif_receive_skb+0x18/0x60
> [ 340.207288] ? process_backlog+0x95/0x140
> [ 340.218948] ? net_rx_action+0x26c/0x3b0
> [ 340.230416] ? __do_softirq+0xc9/0x26a
> [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
> [ 340.253368] </IRQ>
> [ 340.262673] ? do_softirq+0x50/0x60
> [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
> [ 340.285045] ? ip_finish_output2+0x175/0x350
> [ 340.296403] ? ip_finish_output+0x127/0x1d0
> [ 340.307665] ? nf_hook_slow+0x3c/0xb0
> [ 340.318230] ? ip_output+0x72/0xe0
> [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
> [ 340.340070] ? ip_local_out+0x35/0x40
> [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
> [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
> [ 340.372484] ? __skb_clone+0x2e/0x130
> [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
> [ 340.393262] ? tcp_connect+0x938/0xad0
> [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
> [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
> [ 340.424471] ? __inet_stream_connect+0xb3/0x300
> [ 340.435195] ? inet_stream_connect+0x3b/0x60
> [ 340.445607] ? SYSC_connect+0xd9/0x110
> [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
> [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
> [ 340.476636] ? __audit_syscall_exit+0x209/0x290
> [ 340.487151] ? SyS_connect+0xe/0x10
> [ 340.496453] ? do_syscall_64+0x67/0x1b0
> [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
> [ 340.516693] Kernel Offset: disabled
> [ 340.526393] Rebooting in 11 seconds..
>
> This is mostly reliable, and I'm only seeing it on bare metal (not in
> a
> virtualbox vm).
>
> The SELinux skb parse error at the start only sometimes appears, and
> looking at the code, I suspect some kind of memory corruption being
> the
> cause at that point (basic packet header checks).
>
> I bisected the bug down to the following change:
>
> commit bffa72cf7f9df842f0016ba03586039296b4caaf
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Sep 19 05:14:24 2017 -0700
>
> net: sk_buff rbnode reorg
> ...
>
>
> Anyone else able to reproduce this, or have any ideas on what's
> happening?
So far I haven't been able to reproduce with 4.15-rc1 or -linus.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 17:31 ` Stephen Smalley
@ 2017-11-29 17:34 ` Eric Dumazet
2017-11-29 19:29 ` Paul Moore
[not found] ` <CANn89iJc=tZkN41WoCm5Zy9nPfs1tfZf9nuSXYS9EB_aem+y4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 2 replies; 38+ messages in thread
From: Eric Dumazet @ 2017-11-29 17:34 UTC (permalink / raw)
To: Stephen Smalley; +Cc: James Morris, Paul Moore, netdev, selinux
On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
>> I'm seeing a kernel stack corruption bug (detected via gcc) when
>> running
>> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket
>> test:
>>
>> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests
>> /inet_socket/test
>>
>> # Verify that unauthorized client cannot communicate with the
>> server.
>> $result = system
>> "runcon -t test_inet_bad_client_t -- $basedir/client stream
>> 127.0.0.1 65535 2>&1";
>>
>> This correctlly causes an access control error in the Netlabel code,
>> and
>> the bug seems to be triggered during the ICMP send:
>>
>> [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
>> parse packet
>> [ 339.822505] Kernel panic - not syncing: stack-protector: Kernel
>> stack is corrupted in: ffffffff81745af5
>> [ 339.822505]
>> [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-
>> test #15
>> [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
>> FWKT68A 01/19/2017
>> [ 339.885060] Call Trace:
>> [ 339.896875] <IRQ>
>> [ 339.908103] dump_stack+0x63/0x87
>> [ 339.920645] panic+0xe8/0x248
>> [ 339.932668] ? ip_push_pending_frames+0x33/0x40
>> [ 339.946328] ? icmp_send+0x525/0x530
>> [ 339.958861] ? kfree_skbmem+0x60/0x70
>> [ 339.971431] __stack_chk_fail+0x1b/0x20
>> [ 339.984049] icmp_send+0x525/0x530
>> [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
>> [ 340.008997] ? selinux_netlbl_err+0x11/0x20
>> [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
>> [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
>> [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
>> [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
>> [ 340.074562] ? tcp_filter+0x2c/0x40
>> [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
>> [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
>> [ 340.111279] ? ip_local_deliver+0x6f/0xe0
>> [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
>> [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
>> [ 340.147442] ? ip_rcv+0x27c/0x3c0
>> [ 340.158668] ? inet_del_offload+0x40/0x40
>> [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
>> [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
>> [ 340.195282] ? __netif_receive_skb+0x18/0x60
>> [ 340.207288] ? process_backlog+0x95/0x140
>> [ 340.218948] ? net_rx_action+0x26c/0x3b0
>> [ 340.230416] ? __do_softirq+0xc9/0x26a
>> [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
>> [ 340.253368] </IRQ>
>> [ 340.262673] ? do_softirq+0x50/0x60
>> [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
>> [ 340.285045] ? ip_finish_output2+0x175/0x350
>> [ 340.296403] ? ip_finish_output+0x127/0x1d0
>> [ 340.307665] ? nf_hook_slow+0x3c/0xb0
>> [ 340.318230] ? ip_output+0x72/0xe0
>> [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
>> [ 340.340070] ? ip_local_out+0x35/0x40
>> [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
>> [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
>> [ 340.372484] ? __skb_clone+0x2e/0x130
>> [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
>> [ 340.393262] ? tcp_connect+0x938/0xad0
>> [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
>> [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
>> [ 340.424471] ? __inet_stream_connect+0xb3/0x300
>> [ 340.435195] ? inet_stream_connect+0x3b/0x60
>> [ 340.445607] ? SYSC_connect+0xd9/0x110
>> [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
>> [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
>> [ 340.476636] ? __audit_syscall_exit+0x209/0x290
>> [ 340.487151] ? SyS_connect+0xe/0x10
>> [ 340.496453] ? do_syscall_64+0x67/0x1b0
>> [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
>> [ 340.516693] Kernel Offset: disabled
>> [ 340.526393] Rebooting in 11 seconds..
>>
>> This is mostly reliable, and I'm only seeing it on bare metal (not in
>> a
>> virtualbox vm).
>>
>> The SELinux skb parse error at the start only sometimes appears, and
>> looking at the code, I suspect some kind of memory corruption being
>> the
>> cause at that point (basic packet header checks).
>>
>> I bisected the bug down to the following change:
>>
>> commit bffa72cf7f9df842f0016ba03586039296b4caaf
>> Author: Eric Dumazet <edumazet@google.com>
>> Date: Tue Sep 19 05:14:24 2017 -0700
>>
>> net: sk_buff rbnode reorg
>> ...
>>
>>
>> Anyone else able to reproduce this, or have any ideas on what's
>> happening?
>
> So far I haven't been able to reproduce with 4.15-rc1 or -linus.
>
You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
Thanks.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 17:34 ` Eric Dumazet
@ 2017-11-29 19:29 ` Paul Moore
[not found] ` <CANn89iJc=tZkN41WoCm5Zy9nPfs1tfZf9nuSXYS9EB_aem+y4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 0 replies; 38+ messages in thread
From: Paul Moore @ 2017-11-29 19:29 UTC (permalink / raw)
To: James Morris, Eric Dumazet; +Cc: Stephen Smalley, netdev, selinux
On Wed, Nov 29, 2017 at 12:34 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
>>> I'm seeing a kernel stack corruption bug (detected via gcc) when
>>> running
>>> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket
>>> test:
>>>
>>> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests
>>> /inet_socket/test
>>>
>>> # Verify that unauthorized client cannot communicate with the
>>> server.
>>> $result = system
>>> "runcon -t test_inet_bad_client_t -- $basedir/client stream
>>> 127.0.0.1 65535 2>&1";
>>>
>>> This correctlly causes an access control error in the Netlabel code,
>>> and
>>> the bug seems to be triggered during the ICMP send:
>>>
>>> [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
>>> parse packet
>>> [ 339.822505] Kernel panic - not syncing: stack-protector: Kernel
>>> stack is corrupted in: ffffffff81745af5
>>> [ 339.822505]
>>> [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-
>>> test #15
>>> [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
>>> FWKT68A 01/19/2017
>>> [ 339.885060] Call Trace:
>>> [ 339.896875] <IRQ>
>>> [ 339.908103] dump_stack+0x63/0x87
>>> [ 339.920645] panic+0xe8/0x248
>>> [ 339.932668] ? ip_push_pending_frames+0x33/0x40
>>> [ 339.946328] ? icmp_send+0x525/0x530
>>> [ 339.958861] ? kfree_skbmem+0x60/0x70
>>> [ 339.971431] __stack_chk_fail+0x1b/0x20
>>> [ 339.984049] icmp_send+0x525/0x530
...
>>> This is mostly reliable, and I'm only seeing it on bare metal (not in
>>> a
>>> virtualbox vm).
>>>
>>> The SELinux skb parse error at the start only sometimes appears, and
>>> looking at the code, I suspect some kind of memory corruption being
>>> the
>>> cause at that point (basic packet header checks).
>>>
>>> I bisected the bug down to the following change:
>>>
>>> commit bffa72cf7f9df842f0016ba03586039296b4caaf
>>> Author: Eric Dumazet <edumazet@google.com>
>>> Date: Tue Sep 19 05:14:24 2017 -0700
>>>
>>> net: sk_buff rbnode reorg
>>> ...
>>>
>>>
>>> Anyone else able to reproduce this, or have any ideas on what's
>>> happening?
>>
>> So far I haven't been able to reproduce with 4.15-rc1 or -linus.
>
> You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
As another data point, I have not hit this problem either, but I'm not
currently building my test kernels with KASAN enabled.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 17:34 ` Eric Dumazet
@ 2017-11-29 19:59 ` Stephen Smalley
[not found] ` <CANn89iJc=tZkN41WoCm5Zy9nPfs1tfZf9nuSXYS9EB_aem+y4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 0 replies; 38+ messages in thread
From: Stephen Smalley @ 2017-11-29 19:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, selinux-+05T5uksL2qpZYMLLGbcSA
On Wed, 2017-11-29 at 09:34 -0800, Eric Dumazet wrote:
> On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
> wrote:
> > On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
> > > I'm seeing a kernel stack corruption bug (detected via gcc) when
> > > running
> > > the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd
> > > inet_socket
> > > test:
> > >
> > > https://github.com/SELinuxProject/selinux-testsuite/blob/master/t
> > > ests
> > > /inet_socket/test
> > >
> > > # Verify that unauthorized client cannot communicate with the
> > > server.
> > > $result = system
> > > "runcon -t test_inet_bad_client_t -- $basedir/client stream
> > > 127.0.0.1 65535 2>&1";
> > >
> > > This correctlly causes an access control error in the Netlabel
> > > code,
> > > and
> > > the bug seems to be triggered during the ICMP send:
> > >
> > > [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
> > > parse packet
> > > [ 339.822505] Kernel panic - not syncing: stack-protector:
> > > Kernel
> > > stack is corrupted in: ffffffff81745af5
> > > [ 339.822505]
> > > [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-
> > > rc1-
> > > test #15
> > > [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
> > > FWKT68A 01/19/2017
> > > [ 339.885060] Call Trace:
> > > [ 339.896875] <IRQ>
> > > [ 339.908103] dump_stack+0x63/0x87
> > > [ 339.920645] panic+0xe8/0x248
> > > [ 339.932668] ? ip_push_pending_frames+0x33/0x40
> > > [ 339.946328] ? icmp_send+0x525/0x530
> > > [ 339.958861] ? kfree_skbmem+0x60/0x70
> > > [ 339.971431] __stack_chk_fail+0x1b/0x20
> > > [ 339.984049] icmp_send+0x525/0x530
> > > [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
> > > [ 340.008997] ? selinux_netlbl_err+0x11/0x20
> > > [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
> > > [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
> > > [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
> > > [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
> > > [ 340.074562] ? tcp_filter+0x2c/0x40
> > > [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
> > > [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
> > > [ 340.111279] ? ip_local_deliver+0x6f/0xe0
> > > [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
> > > [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
> > > [ 340.147442] ? ip_rcv+0x27c/0x3c0
> > > [ 340.158668] ? inet_del_offload+0x40/0x40
> > > [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
> > > [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
> > > [ 340.195282] ? __netif_receive_skb+0x18/0x60
> > > [ 340.207288] ? process_backlog+0x95/0x140
> > > [ 340.218948] ? net_rx_action+0x26c/0x3b0
> > > [ 340.230416] ? __do_softirq+0xc9/0x26a
> > > [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
> > > [ 340.253368] </IRQ>
> > > [ 340.262673] ? do_softirq+0x50/0x60
> > > [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
> > > [ 340.285045] ? ip_finish_output2+0x175/0x350
> > > [ 340.296403] ? ip_finish_output+0x127/0x1d0
> > > [ 340.307665] ? nf_hook_slow+0x3c/0xb0
> > > [ 340.318230] ? ip_output+0x72/0xe0
> > > [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
> > > [ 340.340070] ? ip_local_out+0x35/0x40
> > > [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
> > > [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
> > > [ 340.372484] ? __skb_clone+0x2e/0x130
> > > [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
> > > [ 340.393262] ? tcp_connect+0x938/0xad0
> > > [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
> > > [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
> > > [ 340.424471] ? __inet_stream_connect+0xb3/0x300
> > > [ 340.435195] ? inet_stream_connect+0x3b/0x60
> > > [ 340.445607] ? SYSC_connect+0xd9/0x110
> > > [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
> > > [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
> > > [ 340.476636] ? __audit_syscall_exit+0x209/0x290
> > > [ 340.487151] ? SyS_connect+0xe/0x10
> > > [ 340.496453] ? do_syscall_64+0x67/0x1b0
> > > [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
> > > [ 340.516693] Kernel Offset: disabled
> > > [ 340.526393] Rebooting in 11 seconds..
> > >
> > > This is mostly reliable, and I'm only seeing it on bare metal
> > > (not in
> > > a
> > > virtualbox vm).
> > >
> > > The SELinux skb parse error at the start only sometimes appears,
> > > and
> > > looking at the code, I suspect some kind of memory corruption
> > > being
> > > the
> > > cause at that point (basic packet header checks).
> > >
> > > I bisected the bug down to the following change:
> > >
> > > commit bffa72cf7f9df842f0016ba03586039296b4caaf
> > > Author: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > > Date: Tue Sep 19 05:14:24 2017 -0700
> > >
> > > net: sk_buff rbnode reorg
> > > ...
> > >
> > >
> > > Anyone else able to reproduce this, or have any ideas on what's
> > > happening?
> >
> > So far I haven't been able to reproduce with 4.15-rc1 or -linus.
> >
>
> You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
Good idea:
==================================================================
BUG:
KASAN: stack-out-of-bounds in __ip_options_echo+0x430/0x5e0
Write of
size 44 at addr ffff8803bc1c7560 by task ksoftirqd/1/16
CPU: 1 PID: 16
Comm: ksoftirqd/1 Not tainted 4.15.0-rc1 #27
Hardware name: Dell Inc.
Latitude E7470/0VNKRJ, BIOS 1.16.4 06/02/2017
Call Trace:
dump_stack+0x5c/0x7c
print_address_description+0x6a/0x280
kasan_report+0x254/0x370
? __ip_options_echo+0x430/0x5e0
memcpy+0x34/0x50
__ip_options_echo+0x430/0x5e0
icmp_send+0x48d/0x7a0
? icmpv4_global_allow+0x50/0x50
?
selinux_netlbl_sk_security_reset+0x20/0x20
? avc_has_perm+0x238/0x260
? avc_has_perm_noaudit+0x1d0/0x1d0
? selinux_peerlbl_enabled+0x50/0x50
? deref_stack_reg+0xd0/0xd0
? __save_stack_trace+0x82/0x100
selinux_socket_sock_rcv_skb+0x341/0x350
?
selinux_sock_rcv_skb_compat+0x200/0x200
? depot_save_stack+0x12f/0x460
? pskb_expand_head+0x9d/0x4d0
? save_stack+0x92/0xa0
?
kasan_kmalloc+0xa0/0xd0
? __kmalloc_node_track_caller+0xf5/0x290
?
skb_copy_and_csum_dev+0x142/0x180
? ip_rcv_finish+0x323/0x690
?
__netif_receive_skb_core+0xe16/0x13d0
? process_backlog+0x10a/0x280
?
net_rx_action+0x3ec/0x5a0
? __do_softirq+0x13f/0x36d
?
__wake_up_common_lock+0xd7/0x130
? tcp_md5_do_lookup+0x27/0x240
security_sock_rcv_skb+0x47/0x60
sk_filter_trim_cap+0x45/0x4b0
?
tcp4_proc_exit+0x11/0x11
tcp_filter+0x5b/0x90
tcp_v4_rcv+0x108a/0x1360
ip_local_deliver_finish+0xf7/0x300
ip_local_deliver+0xf2/0x1a0
? ip_call_ra_chain+0x220/0x220
?
ip_rcv_finish+0x690/0x690
? ip_rcv_finish+0x1b9/0x690
ip_rcv+0x4a6/0x660
? ip_local_deliver+0x1a0/0x1a0
?
inet_del_offload+0x40/0x40
? cpumask_next_and+0x4e/0x70
?
ip_local_deliver+0x1a0/0x1a0
__netif_receive_skb_core+0xe16/0x13d0
?
netdev_info+0x100/0x100
? __accumulate_pelt_segments+0x47/0xd0
?
find_busiest_group+0x1100/0x1100
?
__update_load_avg_se.isra.31+0x34e/0x360
?
__update_load_avg_se.isra.31+0x201/0x360
?
__accumulate_pelt_segments+0x47/0xd0
? process_backlog+0x10a/0x280
process_backlog+0x10a/0x280
net_rx_action+0x3ec/0x5a0
?
napi_complete_done+0x180/0x180
? __schedule+0x4e0/0xd50
?
sched_clock_cpu+0x14/0xe0
__do_softirq+0x13f/0x36d
?
takeover_tasklets+0x2b0/0x2b0
run_ksoftirqd+0x25/0x40
smpboot_thread_fn+0x212/0x2b0
? sort_range+0x20/0x20
?
schedule+0x50/0xc0
kthread+0x174/0x1c0
? sort_range+0x20/0x20
?
kthread_create_worker_on_cpu+0xc0/0xc0
ret_from_fork+0x1f/0x30
The
buggy address belongs to the page:
page:ffffea000ef071c0 count:0
mapcount:0 mapping: (null) index:0x0
flags: 0x17ffffc0000000()
raw: 0017ffffc0000000 0000000000000000 0000000000000000
00000000ffffffff
raw: ffffea000ef071e0 ffffea000ef071e0
0000000000000000 0000000000000000
page dumped because: kasan: bad
access detected
Memory state around the buggy address:
ffff8803bc1c7480: 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00 f4 f2
ffff8803bc1c7500: f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8803bc1c7580: 00 f4 f4 00 00 00 00 00 00 00 00 00 00 00 00 00
^
ffff8803bc1c7600: 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2
f2 f2 f2 00 f4
ffff8803bc1c7680: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2
f2 f2 00 00
==================================================================
Disabling lock debugging due to kernel taint
SELinux: failure in
selinux_parse_skb(), unable to parse packet
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-29 19:59 ` Stephen Smalley
0 siblings, 0 replies; 38+ messages in thread
From: Stephen Smalley @ 2017-11-29 19:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: James Morris, Paul Moore, netdev, selinux
On Wed, 2017-11-29 at 09:34 -0800, Eric Dumazet wrote:
> On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds@tycho.nsa.gov>
> wrote:
> > On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
> > > I'm seeing a kernel stack corruption bug (detected via gcc) when
> > > running
> > > the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd
> > > inet_socket
> > > test:
> > >
> > > https://github.com/SELinuxProject/selinux-testsuite/blob/master/t
> > > ests
> > > /inet_socket/test
> > >
> > > # Verify that unauthorized client cannot communicate with the
> > > server.
> > > $result = system
> > > "runcon -t test_inet_bad_client_t -- $basedir/client stream
> > > 127.0.0.1 65535 2>&1";
> > >
> > > This correctlly causes an access control error in the Netlabel
> > > code,
> > > and
> > > the bug seems to be triggered during the ICMP send:
> > >
> > > [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
> > > parse packet
> > > [ 339.822505] Kernel panic - not syncing: stack-protector:
> > > Kernel
> > > stack is corrupted in: ffffffff81745af5
> > > [ 339.822505]
> > > [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-
> > > rc1-
> > > test #15
> > > [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
> > > FWKT68A 01/19/2017
> > > [ 339.885060] Call Trace:
> > > [ 339.896875] <IRQ>
> > > [ 339.908103] dump_stack+0x63/0x87
> > > [ 339.920645] panic+0xe8/0x248
> > > [ 339.932668] ? ip_push_pending_frames+0x33/0x40
> > > [ 339.946328] ? icmp_send+0x525/0x530
> > > [ 339.958861] ? kfree_skbmem+0x60/0x70
> > > [ 339.971431] __stack_chk_fail+0x1b/0x20
> > > [ 339.984049] icmp_send+0x525/0x530
> > > [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
> > > [ 340.008997] ? selinux_netlbl_err+0x11/0x20
> > > [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
> > > [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
> > > [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
> > > [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
> > > [ 340.074562] ? tcp_filter+0x2c/0x40
> > > [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
> > > [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
> > > [ 340.111279] ? ip_local_deliver+0x6f/0xe0
> > > [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
> > > [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
> > > [ 340.147442] ? ip_rcv+0x27c/0x3c0
> > > [ 340.158668] ? inet_del_offload+0x40/0x40
> > > [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
> > > [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
> > > [ 340.195282] ? __netif_receive_skb+0x18/0x60
> > > [ 340.207288] ? process_backlog+0x95/0x140
> > > [ 340.218948] ? net_rx_action+0x26c/0x3b0
> > > [ 340.230416] ? __do_softirq+0xc9/0x26a
> > > [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
> > > [ 340.253368] </IRQ>
> > > [ 340.262673] ? do_softirq+0x50/0x60
> > > [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
> > > [ 340.285045] ? ip_finish_output2+0x175/0x350
> > > [ 340.296403] ? ip_finish_output+0x127/0x1d0
> > > [ 340.307665] ? nf_hook_slow+0x3c/0xb0
> > > [ 340.318230] ? ip_output+0x72/0xe0
> > > [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
> > > [ 340.340070] ? ip_local_out+0x35/0x40
> > > [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
> > > [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
> > > [ 340.372484] ? __skb_clone+0x2e/0x130
> > > [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
> > > [ 340.393262] ? tcp_connect+0x938/0xad0
> > > [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
> > > [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
> > > [ 340.424471] ? __inet_stream_connect+0xb3/0x300
> > > [ 340.435195] ? inet_stream_connect+0x3b/0x60
> > > [ 340.445607] ? SYSC_connect+0xd9/0x110
> > > [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
> > > [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
> > > [ 340.476636] ? __audit_syscall_exit+0x209/0x290
> > > [ 340.487151] ? SyS_connect+0xe/0x10
> > > [ 340.496453] ? do_syscall_64+0x67/0x1b0
> > > [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
> > > [ 340.516693] Kernel Offset: disabled
> > > [ 340.526393] Rebooting in 11 seconds..
> > >
> > > This is mostly reliable, and I'm only seeing it on bare metal
> > > (not in
> > > a
> > > virtualbox vm).
> > >
> > > The SELinux skb parse error at the start only sometimes appears,
> > > and
> > > looking at the code, I suspect some kind of memory corruption
> > > being
> > > the
> > > cause at that point (basic packet header checks).
> > >
> > > I bisected the bug down to the following change:
> > >
> > > commit bffa72cf7f9df842f0016ba03586039296b4caaf
> > > Author: Eric Dumazet <edumazet@google.com>
> > > Date: Tue Sep 19 05:14:24 2017 -0700
> > >
> > > net: sk_buff rbnode reorg
> > > ...
> > >
> > >
> > > Anyone else able to reproduce this, or have any ideas on what's
> > > happening?
> >
> > So far I haven't been able to reproduce with 4.15-rc1 or -linus.
> >
>
> You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
Good idea:
==================================================================
BUG:
KASAN: stack-out-of-bounds in __ip_options_echo+0x430/0x5e0
Write of
size 44 at addr ffff8803bc1c7560 by task ksoftirqd/1/16
CPU: 1 PID: 16
Comm: ksoftirqd/1 Not tainted 4.15.0-rc1 #27
Hardware name: Dell Inc.
Latitude E7470/0VNKRJ, BIOS 1.16.4 06/02/2017
Call Trace:
dump_stack+0x5c/0x7c
print_address_description+0x6a/0x280
kasan_report+0x254/0x370
? __ip_options_echo+0x430/0x5e0
memcpy+0x34/0x50
__ip_options_echo+0x430/0x5e0
icmp_send+0x48d/0x7a0
? icmpv4_global_allow+0x50/0x50
?
selinux_netlbl_sk_security_reset+0x20/0x20
? avc_has_perm+0x238/0x260
? avc_has_perm_noaudit+0x1d0/0x1d0
? selinux_peerlbl_enabled+0x50/0x50
? deref_stack_reg+0xd0/0xd0
? __save_stack_trace+0x82/0x100
selinux_socket_sock_rcv_skb+0x341/0x350
?
selinux_sock_rcv_skb_compat+0x200/0x200
? depot_save_stack+0x12f/0x460
? pskb_expand_head+0x9d/0x4d0
? save_stack+0x92/0xa0
?
kasan_kmalloc+0xa0/0xd0
? __kmalloc_node_track_caller+0xf5/0x290
?
skb_copy_and_csum_dev+0x142/0x180
? ip_rcv_finish+0x323/0x690
?
__netif_receive_skb_core+0xe16/0x13d0
? process_backlog+0x10a/0x280
?
net_rx_action+0x3ec/0x5a0
? __do_softirq+0x13f/0x36d
?
__wake_up_common_lock+0xd7/0x130
? tcp_md5_do_lookup+0x27/0x240
security_sock_rcv_skb+0x47/0x60
sk_filter_trim_cap+0x45/0x4b0
?
tcp4_proc_exit+0x11/0x11
tcp_filter+0x5b/0x90
tcp_v4_rcv+0x108a/0x1360
ip_local_deliver_finish+0xf7/0x300
ip_local_deliver+0xf2/0x1a0
? ip_call_ra_chain+0x220/0x220
?
ip_rcv_finish+0x690/0x690
? ip_rcv_finish+0x1b9/0x690
ip_rcv+0x4a6/0x660
? ip_local_deliver+0x1a0/0x1a0
?
inet_del_offload+0x40/0x40
? cpumask_next_and+0x4e/0x70
?
ip_local_deliver+0x1a0/0x1a0
__netif_receive_skb_core+0xe16/0x13d0
?
netdev_info+0x100/0x100
? __accumulate_pelt_segments+0x47/0xd0
?
find_busiest_group+0x1100/0x1100
?
__update_load_avg_se.isra.31+0x34e/0x360
?
__update_load_avg_se.isra.31+0x201/0x360
?
__accumulate_pelt_segments+0x47/0xd0
? process_backlog+0x10a/0x280
process_backlog+0x10a/0x280
net_rx_action+0x3ec/0x5a0
?
napi_complete_done+0x180/0x180
? __schedule+0x4e0/0xd50
?
sched_clock_cpu+0x14/0xe0
__do_softirq+0x13f/0x36d
?
takeover_tasklets+0x2b0/0x2b0
run_ksoftirqd+0x25/0x40
smpboot_thread_fn+0x212/0x2b0
? sort_range+0x20/0x20
?
schedule+0x50/0xc0
kthread+0x174/0x1c0
? sort_range+0x20/0x20
?
kthread_create_worker_on_cpu+0xc0/0xc0
ret_from_fork+0x1f/0x30
The
buggy address belongs to the page:
page:ffffea000ef071c0 count:0
mapcount:0 mapping: (null) index:0x0
flags: 0x17ffffc0000000()
raw: 0017ffffc0000000 0000000000000000 0000000000000000
00000000ffffffff
raw: ffffea000ef071e0 ffffea000ef071e0
0000000000000000 0000000000000000
page dumped because: kasan: bad
access detected
Memory state around the buggy address:
ffff8803bc1c7480: 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00 f4 f2
ffff8803bc1c7500: f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8803bc1c7580: 00 f4 f4 00 00 00 00 00 00 00 00 00 00 00 00 00
^
ffff8803bc1c7600: 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2
f2 f2 f2 00 f4
ffff8803bc1c7680: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2
f2 f2 00 00
==================================================================
Disabling lock debugging due to kernel taint
SELinux: failure in
selinux_parse_skb(), unable to parse packet
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 19:59 ` Stephen Smalley
(?)
@ 2017-11-29 20:23 ` Eric Dumazet
2017-11-29 22:49 ` Eric Dumazet
-1 siblings, 1 reply; 38+ messages in thread
From: Eric Dumazet @ 2017-11-29 20:23 UTC (permalink / raw)
To: Stephen Smalley; +Cc: James Morris, Paul Moore, netdev, selinux
On Wed, Nov 29, 2017 at 11:59 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2017-11-29 at 09:34 -0800, Eric Dumazet wrote:
>> On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds@tycho.nsa.gov>
>> wrote:
>> > On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
>> > > I'm seeing a kernel stack corruption bug (detected via gcc) when
>> > > running
>> > > the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd
>> > > inet_socket
>> > > test:
>> > >
>> > > https://github.com/SELinuxProject/selinux-testsuite/blob/master/t
>> > > ests
>> > > /inet_socket/test
>> > >
>> > > # Verify that unauthorized client cannot communicate with the
>> > > server.
>> > > $result = system
>> > > "runcon -t test_inet_bad_client_t -- $basedir/client stream
>> > > 127.0.0.1 65535 2>&1";
>> > >
>> > > This correctlly causes an access control error in the Netlabel
>> > > code,
>> > > and
>> > > the bug seems to be triggered during the ICMP send:
>> > >
>> > > [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
>> > > parse packet
>> > > [ 339.822505] Kernel panic - not syncing: stack-protector:
>> > > Kernel
>> > > stack is corrupted in: ffffffff81745af5
>> > > [ 339.822505]
>> > > [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-
>> > > rc1-
>> > > test #15
>> > > [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
>> > > FWKT68A 01/19/2017
>> > > [ 339.885060] Call Trace:
>> > > [ 339.896875] <IRQ>
>> > > [ 339.908103] dump_stack+0x63/0x87
>> > > [ 339.920645] panic+0xe8/0x248
>> > > [ 339.932668] ? ip_push_pending_frames+0x33/0x40
>> > > [ 339.946328] ? icmp_send+0x525/0x530
>> > > [ 339.958861] ? kfree_skbmem+0x60/0x70
>> > > [ 339.971431] __stack_chk_fail+0x1b/0x20
>> > > [ 339.984049] icmp_send+0x525/0x530
>> > > [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
>> > > [ 340.008997] ? selinux_netlbl_err+0x11/0x20
>> > > [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
>> > > [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
>> > > [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
>> > > [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
>> > > [ 340.074562] ? tcp_filter+0x2c/0x40
>> > > [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
>> > > [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
>> > > [ 340.111279] ? ip_local_deliver+0x6f/0xe0
>> > > [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
>> > > [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
>> > > [ 340.147442] ? ip_rcv+0x27c/0x3c0
>> > > [ 340.158668] ? inet_del_offload+0x40/0x40
>> > > [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
>> > > [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
>> > > [ 340.195282] ? __netif_receive_skb+0x18/0x60
>> > > [ 340.207288] ? process_backlog+0x95/0x140
>> > > [ 340.218948] ? net_rx_action+0x26c/0x3b0
>> > > [ 340.230416] ? __do_softirq+0xc9/0x26a
>> > > [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
>> > > [ 340.253368] </IRQ>
>> > > [ 340.262673] ? do_softirq+0x50/0x60
>> > > [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
>> > > [ 340.285045] ? ip_finish_output2+0x175/0x350
>> > > [ 340.296403] ? ip_finish_output+0x127/0x1d0
>> > > [ 340.307665] ? nf_hook_slow+0x3c/0xb0
>> > > [ 340.318230] ? ip_output+0x72/0xe0
>> > > [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
>> > > [ 340.340070] ? ip_local_out+0x35/0x40
>> > > [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
>> > > [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
>> > > [ 340.372484] ? __skb_clone+0x2e/0x130
>> > > [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
>> > > [ 340.393262] ? tcp_connect+0x938/0xad0
>> > > [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
>> > > [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
>> > > [ 340.424471] ? __inet_stream_connect+0xb3/0x300
>> > > [ 340.435195] ? inet_stream_connect+0x3b/0x60
>> > > [ 340.445607] ? SYSC_connect+0xd9/0x110
>> > > [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
>> > > [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
>> > > [ 340.476636] ? __audit_syscall_exit+0x209/0x290
>> > > [ 340.487151] ? SyS_connect+0xe/0x10
>> > > [ 340.496453] ? do_syscall_64+0x67/0x1b0
>> > > [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
>> > > [ 340.516693] Kernel Offset: disabled
>> > > [ 340.526393] Rebooting in 11 seconds..
>> > >
>> > > This is mostly reliable, and I'm only seeing it on bare metal
>> > > (not in
>> > > a
>> > > virtualbox vm).
>> > >
>> > > The SELinux skb parse error at the start only sometimes appears,
>> > > and
>> > > looking at the code, I suspect some kind of memory corruption
>> > > being
>> > > the
>> > > cause at that point (basic packet header checks).
>> > >
>> > > I bisected the bug down to the following change:
>> > >
>> > > commit bffa72cf7f9df842f0016ba03586039296b4caaf
>> > > Author: Eric Dumazet <edumazet@google.com>
>> > > Date: Tue Sep 19 05:14:24 2017 -0700
>> > >
>> > > net: sk_buff rbnode reorg
>> > > ...
>> > >
>> > >
>> > > Anyone else able to reproduce this, or have any ideas on what's
>> > > happening?
>> >
>> > So far I haven't been able to reproduce with 4.15-rc1 or -linus.
>> >
>>
>> You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
>
> Good idea:
>
> ==================================================================
> BUG:
> KASAN: stack-out-of-bounds in __ip_options_echo+0x430/0x5e0
> Write of
> size 44 at addr ffff8803bc1c7560 by task ksoftirqd/1/16
>
> CPU: 1 PID: 16
> Comm: ksoftirqd/1 Not tainted 4.15.0-rc1 #27
> Hardware name: Dell Inc.
> Latitude E7470/0VNKRJ, BIOS 1.16.4 06/02/2017
> Call Trace:
>
> dump_stack+0x5c/0x7c
> print_address_description+0x6a/0x280
>
> kasan_report+0x254/0x370
> ? __ip_options_echo+0x430/0x5e0
>
> memcpy+0x34/0x50
> __ip_options_echo+0x430/0x5e0
> icmp_send+0x48d/0x7a0
>
> ? icmpv4_global_allow+0x50/0x50
> ?
> selinux_netlbl_sk_security_reset+0x20/0x20
> ? avc_has_perm+0x238/0x260
>
> ? avc_has_perm_noaudit+0x1d0/0x1d0
> ? selinux_peerlbl_enabled+0x50/0x50
> ? deref_stack_reg+0xd0/0xd0
> ? __save_stack_trace+0x82/0x100
>
> selinux_socket_sock_rcv_skb+0x341/0x350
> ?
> selinux_sock_rcv_skb_compat+0x200/0x200
> ? depot_save_stack+0x12f/0x460
> ? pskb_expand_head+0x9d/0x4d0
> ? save_stack+0x92/0xa0
> ?
> kasan_kmalloc+0xa0/0xd0
> ? __kmalloc_node_track_caller+0xf5/0x290
> ?
> skb_copy_and_csum_dev+0x142/0x180
> ? ip_rcv_finish+0x323/0x690
> ?
> __netif_receive_skb_core+0xe16/0x13d0
> ? process_backlog+0x10a/0x280
> ?
> net_rx_action+0x3ec/0x5a0
> ? __do_softirq+0x13f/0x36d
> ?
> __wake_up_common_lock+0xd7/0x130
> ? tcp_md5_do_lookup+0x27/0x240
>
> security_sock_rcv_skb+0x47/0x60
> sk_filter_trim_cap+0x45/0x4b0
> ?
> tcp4_proc_exit+0x11/0x11
> tcp_filter+0x5b/0x90
>
> tcp_v4_rcv+0x108a/0x1360
> ip_local_deliver_finish+0xf7/0x300
>
> ip_local_deliver+0xf2/0x1a0
> ? ip_call_ra_chain+0x220/0x220
> ?
> ip_rcv_finish+0x690/0x690
> ? ip_rcv_finish+0x1b9/0x690
>
> ip_rcv+0x4a6/0x660
> ? ip_local_deliver+0x1a0/0x1a0
> ?
> inet_del_offload+0x40/0x40
> ? cpumask_next_and+0x4e/0x70
> ?
> ip_local_deliver+0x1a0/0x1a0
> __netif_receive_skb_core+0xe16/0x13d0
> ?
> netdev_info+0x100/0x100
> ? __accumulate_pelt_segments+0x47/0xd0
> ?
> find_busiest_group+0x1100/0x1100
> ?
> __update_load_avg_se.isra.31+0x34e/0x360
> ?
> __update_load_avg_se.isra.31+0x201/0x360
> ?
> __accumulate_pelt_segments+0x47/0xd0
> ? process_backlog+0x10a/0x280
>
> process_backlog+0x10a/0x280
> net_rx_action+0x3ec/0x5a0
> ?
> napi_complete_done+0x180/0x180
> ? __schedule+0x4e0/0xd50
> ?
> sched_clock_cpu+0x14/0xe0
> __do_softirq+0x13f/0x36d
> ?
> takeover_tasklets+0x2b0/0x2b0
> run_ksoftirqd+0x25/0x40
>
> smpboot_thread_fn+0x212/0x2b0
> ? sort_range+0x20/0x20
> ?
> schedule+0x50/0xc0
> kthread+0x174/0x1c0
> ? sort_range+0x20/0x20
> ?
> kthread_create_worker_on_cpu+0xc0/0xc0
> ret_from_fork+0x1f/0x30
>
> The
> buggy address belongs to the page:
> page:ffffea000ef071c0 count:0
> mapcount:0 mapping: (null) index:0x0
> flags: 0x17ffffc0000000()
> raw: 0017ffffc0000000 0000000000000000 0000000000000000
> 00000000ffffffff
> raw: ffffea000ef071e0 ffffea000ef071e0
> 0000000000000000 0000000000000000
> page dumped because: kasan: bad
> access detected
>
> Memory state around the buggy address:
>
> ffff8803bc1c7480: 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00 f4 f2
>
> ffff8803bc1c7500: f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00
>
>>ffff8803bc1c7580: 00 f4 f4 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> ^
> ffff8803bc1c7600: 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2
> f2 f2 f2 00 f4
> ffff8803bc1c7680: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2
> f2 f2 00 00
>
> ==================================================================
>
> Disabling lock debugging due to kernel taint
> SELinux: failure in
> selinux_parse_skb(), unable to parse packet
I suspect this exposes an ancient bug, caused by fact that TCP moves
IP[6]CB in skb->cb[]
Basically the 2nd tcp_filter() added in commit
8fac365f63c866a00015fa13932d8ffc584518b8
("tcp: Add a tcp_filter hook before handle ack packet") was not
expecting selinux code being called a 2nd time,
while skb->cb[] has been mangled [1]
[1]
memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
sizeof(struct inet_skb_parm));
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 20:23 ` Eric Dumazet
@ 2017-11-29 22:49 ` Eric Dumazet
2017-11-29 23:41 ` James Morris
0 siblings, 1 reply; 38+ messages in thread
From: Eric Dumazet @ 2017-11-29 22:49 UTC (permalink / raw)
To: Eric Dumazet, Stephen Smalley; +Cc: James Morris, Paul Moore, netdev, selinux
On Wed, 2017-11-29 at 12:23 -0800, Eric Dumazet wrote:
>
> I suspect this exposes an ancient bug, caused by fact that TCP moves
> IP[6]CB in skb->cb[]
>
> Basically the 2nd tcp_filter() added in commit
> 8fac365f63c866a00015fa13932d8ffc584518b8
> ("tcp: Add a tcp_filter hook before handle ack packet") was not
> expecting selinux code being called a 2nd time,
> while skb->cb[] has been mangled [1]
>
> [1]
> memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> sizeof(struct inet_skb_parm));
Please try this fix for IPv4 (a similar patch will be needed for IPv6)
net/ipv4/tcp_ipv4.c | 51 ++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..912928105942b9714dda9132e45961ab1baf0852 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1591,6 +1591,28 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL(tcp_filter);
+static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
+ const struct tcphdr *th)
+{
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
+ sizeof(struct inet_skb_parm));
+ barrier();
+
+ TCP_SKB_CB(skb)->seq = ntohl(th->seq);
+ TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
+ skb->len - th->doff * 4);
+ TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
+ TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
+ TCP_SKB_CB(skb)->tcp_tw_isn = 0;
+ TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
+ TCP_SKB_CB(skb)->sacked = 0;
+ TCP_SKB_CB(skb)->has_rxtstamp =
+ skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
+}
+
/*
* From tcp_input.c
*/
@@ -1631,24 +1653,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
- /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
- * barrier() makes sure compiler wont play fool^Waliasing games.
- */
- memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
- sizeof(struct inet_skb_parm));
- barrier();
-
- TCP_SKB_CB(skb)->seq = ntohl(th->seq);
- TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff * 4);
- TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
- TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
- TCP_SKB_CB(skb)->tcp_tw_isn = 0;
- TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
- TCP_SKB_CB(skb)->sacked = 0;
- TCP_SKB_CB(skb)->has_rxtstamp =
- skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
-
lookup:
sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
th->dest, sdif, &refcounted);
@@ -1679,8 +1683,12 @@ int tcp_v4_rcv(struct sk_buff *skb)
sock_hold(sk);
refcounted = true;
nsk = NULL;
- if (!tcp_filter(sk, skb))
+ if (!tcp_filter(sk, skb)) {
+ th = (const struct tcphdr *)skb->data;
+ iph = ip_hdr(skb);
+ tcp_v4_fill_cb(skb, iph, th);
nsk = tcp_check_req(sk, skb, req, false);
+ }
if (!nsk) {
reqsk_put(req);
goto discard_and_relse;
@@ -1712,6 +1720,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_and_relse;
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
+ tcp_v4_fill_cb(skb, iph, th);
skb->dev = NULL;
@@ -1742,6 +1751,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
+ tcp_v4_fill_cb(skb, iph, th);
+
if (tcp_checksum_complete(skb)) {
csum_error:
__TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
@@ -1768,6 +1779,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_it;
}
+ tcp_v4_fill_cb(skb, iph, th);
+
if (tcp_checksum_complete(skb)) {
inet_twsk_put(inet_twsk(sk));
goto csum_error;
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 22:49 ` Eric Dumazet
@ 2017-11-29 23:41 ` James Morris
0 siblings, 0 replies; 38+ messages in thread
From: James Morris @ 2017-11-29 23:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, Stephen Smalley, Paul Moore, netdev, selinux
[-- Attachment #1: Type: text/plain, Size: 973 bytes --]
On Wed, 29 Nov 2017, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 12:23 -0800, Eric Dumazet wrote:
> >
> > I suspect this exposes an ancient bug, caused by fact that TCP moves
> > IP[6]CB in skb->cb[]
> >
> > Basically the 2nd tcp_filter() added in commit
> > 8fac365f63c866a00015fa13932d8ffc584518b8
> > ("tcp: Add a tcp_filter hook before handle ack packet") was not
> > expecting selinux code being called a 2nd time,
> > while skb->cb[] has been mangled [1]
> >
> > [1]
> > memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> > sizeof(struct inet_skb_parm));
>
> Please try this fix for IPv4 (a similar patch will be needed for IPv6)
>
> net/ipv4/tcp_ipv4.c | 51 ++++++++++++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 19 deletions(-)
Works for me, no crashes with the testsuite running in a loop.
Tested-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-29 10:26 [BUG] kernel stack corruption during/after Netlabel error James Morris
@ 2017-11-30 0:22 ` Casey Schaufler
2017-11-29 17:31 ` Stephen Smalley
2017-11-30 0:22 ` Casey Schaufler
2 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 0:22 UTC (permalink / raw)
To: James Morris, Paul Moore, Eric Dumazet
Cc: netdev, Stephen Smalley, selinux, LSM
On 11/29/2017 2:26 AM, James Morris wrote:
> I'm seeing a kernel stack corruption bug (detected via gcc) when running
> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket test:
>
> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests/inet_socket/test
>
> # Verify that unauthorized client cannot communicate with the server.
> $result = system
> "runcon -t test_inet_bad_client_t -- $basedir/client stream 127.0.0.1 65535 2>&1";
>
> This correctlly causes an access control error in the Netlabel code, and
> the bug seems to be triggered during the ICMP send:
>
> ...<SNIP>...
>
> This is mostly reliable, and I'm only seeing it on bare metal (not in a
> virtualbox vm).
>
> The SELinux skb parse error at the start only sometimes appears, and
> looking at the code, I suspect some kind of memory corruption being the
> cause at that point (basic packet header checks).
>
> I bisected the bug down to the following change:
>
> commit bffa72cf7f9df842f0016ba03586039296b4caaf
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Sep 19 05:14:24 2017 -0700
>
> net: sk_buff rbnode reorg
> ...
>
>
> Anyone else able to reproduce this, or have any ideas on what's happening?
I have also bisected a problem to this change. I do not have a trace
because the problem manifests as a hard system hang without a trace
being presented. The issue arises when Smack attempts to relabel a TCP
socket using netlbl_sock_setattr().
I see that there is a proposed fix later in the thread, but I don't see
the patch. Could you send it to me, so I can try it on my problem?
Thank you.
>
>
>
> - James
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 0:22 ` Casey Schaufler
0 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 0:22 UTC (permalink / raw)
To: linux-security-module
On 11/29/2017 2:26 AM, James Morris wrote:
> I'm seeing a kernel stack corruption bug (detected via gcc) when running
> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket test:
>
> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests/inet_socket/test
>
> # Verify that unauthorized client cannot communicate with the server.
> $result = system
> "runcon -t test_inet_bad_client_t -- $basedir/client stream 127.0.0.1 65535 2>&1";
>
> This correctlly causes an access control error in the Netlabel code, and
> the bug seems to be triggered during the ICMP send:
>
> ...<SNIP>...
>
> This is mostly reliable, and I'm only seeing it on bare metal (not in a
> virtualbox vm).
>
> The SELinux skb parse error at the start only sometimes appears, and
> looking at the code, I suspect some kind of memory corruption being the
> cause at that point (basic packet header checks).
>
> I bisected the bug down to the following change:
>
> commit bffa72cf7f9df842f0016ba03586039296b4caaf
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Sep 19 05:14:24 2017 -0700
>
> net: sk_buff rbnode reorg
> ...
>
>
> Anyone else able to reproduce this, or have any ideas on what's happening?
I have also bisected a problem to this change. I do not have a trace
because the problem manifests as a hard system hang without a trace
being presented. The issue arises when Smack attempts to relabel a TCP
socket using netlbl_sock_setattr().
I see that there is a proposed fix later in the thread, but I don't see
the patch. Could you send it to me, so I can try it on my problem?
Thank you.
>
>
>
> - James
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 0:22 ` Casey Schaufler
@ 2017-11-30 0:31 ` James Morris
-1 siblings, 0 replies; 38+ messages in thread
From: James Morris @ 2017-11-30 0:31 UTC (permalink / raw)
To: Casey Schaufler
Cc: Paul Moore, Eric Dumazet, netdev, Stephen Smalley, selinux, LSM
On Wed, 29 Nov 2017, Casey Schaufler wrote:
> I see that there is a proposed fix later in the thread, but I don't see
> the patch. Could you send it to me, so I can try it on my problem?
Forwarded off-list.
Interestingly, I didn't see the KASAN output email from Stephen here.
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 0:31 ` James Morris
0 siblings, 0 replies; 38+ messages in thread
From: James Morris @ 2017-11-30 0:31 UTC (permalink / raw)
To: linux-security-module
On Wed, 29 Nov 2017, Casey Schaufler wrote:
> I see that there is a proposed fix later in the thread, but I don't see
> the patch. Could you send it to me, so I can try it on my problem?
Forwarded off-list.
Interestingly, I didn't see the KASAN output email from Stephen here.
--
James Morris
<james.l.morris@oracle.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 0:31 ` James Morris
@ 2017-11-30 3:16 ` Casey Schaufler
-1 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 3:16 UTC (permalink / raw)
To: James Morris
Cc: Paul Moore, Eric Dumazet, netdev, Stephen Smalley, selinux, LSM
On 11/29/2017 4:31 PM, James Morris wrote:
> On Wed, 29 Nov 2017, Casey Schaufler wrote:
>
>> I see that there is a proposed fix later in the thread, but I don't see
>> the patch. Could you send it to me, so I can try it on my problem?
> Forwarded off-list.
The patch does fix the problem I was seeing in Smack.
>
> Interestingly, I didn't see the KASAN output email from Stephen here.
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 3:16 ` Casey Schaufler
0 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 3:16 UTC (permalink / raw)
To: linux-security-module
On 11/29/2017 4:31 PM, James Morris wrote:
> On Wed, 29 Nov 2017, Casey Schaufler wrote:
>
>> I see that there is a proposed fix later in the thread, but I don't see
>> the patch. Could you send it to me, so I can try it on my problem?
> Forwarded off-list.
The patch does fix the problem I was seeing in Smack.
>
> Interestingly, I didn't see the KASAN output email from Stephen here.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 3:16 ` Casey Schaufler
@ 2017-11-30 10:50 ` Eric Dumazet
-1 siblings, 0 replies; 38+ messages in thread
From: Eric Dumazet @ 2017-11-30 10:50 UTC (permalink / raw)
To: Casey Schaufler, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
> On 11/29/2017 4:31 PM, James Morris wrote:
> > On Wed, 29 Nov 2017, Casey Schaufler wrote:
> >
> > > I see that there is a proposed fix later in the thread, but I
> > > don't see
> > > the patch. Could you send it to me, so I can try it on my
> > > problem?
> >
> > Forwarded off-list.
>
> The patch does fix the problem I was seeing in Smack.
Can you guys test the following more complete patch ?
It should cover IPv4 and IPv6, and also the corner cases.
( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
42844/ that I spotted while cooking this patch )
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL(tcp_filter);
+static void tcp_v4_restore_cb(struct sk_buff *skb)
+{
+ memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
+ sizeof(struct inet_skb_parm));
+}
+
+static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
+ const struct tcphdr *th)
+{
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
+ sizeof(struct inet_skb_parm));
+ barrier();
+
+ TCP_SKB_CB(skb)->seq = ntohl(th->seq);
+ TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
+ skb->len - th->doff * 4);
+ TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
+ TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
+ TCP_SKB_CB(skb)->tcp_tw_isn = 0;
+ TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
+ TCP_SKB_CB(skb)->sacked = 0;
+ TCP_SKB_CB(skb)->has_rxtstamp =
+ skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
+}
+
/*
* From tcp_input.c
*/
@@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
- /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
- * barrier() makes sure compiler wont play fool^Waliasing games.
- */
- memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
- sizeof(struct inet_skb_parm));
- barrier();
-
- TCP_SKB_CB(skb)->seq = ntohl(th->seq);
- TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff * 4);
- TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
- TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
- TCP_SKB_CB(skb)->tcp_tw_isn = 0;
- TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
- TCP_SKB_CB(skb)->sacked = 0;
- TCP_SKB_CB(skb)->has_rxtstamp =
- skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
-
lookup:
sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
th->dest, sdif, &refcounted);
@@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
sock_hold(sk);
refcounted = true;
nsk = NULL;
- if (!tcp_filter(sk, skb))
+ if (!tcp_filter(sk, skb)) {
+ th = (const struct tcphdr *)skb->data;
+ iph = ip_hdr(skb);
+ tcp_v4_fill_cb(skb, iph, th);
nsk = tcp_check_req(sk, skb, req, false);
+ }
if (!nsk) {
reqsk_put(req);
goto discard_and_relse;
}
if (nsk == sk) {
reqsk_put(req);
+ tcp_v4_restore_cb(skb);
} else if (tcp_child_process(sk, nsk, skb)) {
tcp_v4_send_reset(nsk, skb);
goto discard_and_relse;
@@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_and_relse;
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
+ tcp_v4_fill_cb(skb, iph, th);
skb->dev = NULL;
@@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
+ tcp_v4_fill_cb(skb, iph, th);
+
if (tcp_checksum_complete(skb)) {
csum_error:
__TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
@@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_it;
}
+ tcp_v4_fill_cb(skb, iph, th);
+
if (tcp_checksum_complete(skb)) {
inet_twsk_put(inet_twsk(sk));
goto csum_error;
@@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
if (sk2) {
inet_twsk_deschedule_put(inet_twsk(sk));
sk = sk2;
+ tcp_v4_restore_cb(skb);
refcounted = false;
goto process;
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
struct sock *nsk;
sk = req->rsk_listener;
- tcp_v6_fill_cb(skb, hdr, th);
if (tcp_v6_inbound_md5_hash(sk, skb)) {
sk_drops_add(sk, skb);
reqsk_put(req);
@@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
sock_hold(sk);
refcounted = true;
nsk = NULL;
- if (!tcp_filter(sk, skb))
+ if (!tcp_filter(sk, skb)) {
+ th = (const struct tcphdr *)skb->data;
+ hdr = ipv6_hdr(skb);
+ tcp_v6_fill_cb(skb, hdr, th);
nsk = tcp_check_req(sk, skb, req, false);
+ }
if (!nsk) {
reqsk_put(req);
goto discard_and_relse;
@@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_and_relse;
- tcp_v6_fill_cb(skb, hdr, th);
-
if (tcp_v6_inbound_md5_hash(sk, skb))
goto discard_and_relse;
@@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
goto discard_and_relse;
th = (const struct tcphdr *)skb->data;
hdr = ipv6_hdr(skb);
+ tcp_v6_fill_cb(skb, hdr, th);
skb->dev = NULL;
@@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
tcp_v6_timewait_ack(sk, skb);
break;
case TCP_TW_RST:
- tcp_v6_restore_cb(skb);
tcp_v6_send_reset(sk, skb);
inet_twsk_deschedule_put(inet_twsk(sk));
goto discard_it;
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 10:50 ` Eric Dumazet
0 siblings, 0 replies; 38+ messages in thread
From: Eric Dumazet @ 2017-11-30 10:50 UTC (permalink / raw)
To: linux-security-module
On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
> On 11/29/2017 4:31 PM, James Morris wrote:
> > On Wed, 29 Nov 2017, Casey Schaufler wrote:
> >
> > > I see that there is a proposed fix later in the thread, but I
> > > don't see
> > > the patch. Could you send it to me, so I can try it on my
> > > problem?
> >
> > Forwarded off-list.
>
> The patch does fix the problem I was seeing in Smack.
Can you guys test the following more complete patch ?
It should cover IPv4 and IPv6, and also the corner cases.
( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
42844/ that I spotted while cooking this patch )
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL(tcp_filter);
+static void tcp_v4_restore_cb(struct sk_buff *skb)
+{
+ memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
+ sizeof(struct inet_skb_parm));
+}
+
+static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
+ const struct tcphdr *th)
+{
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
+ sizeof(struct inet_skb_parm));
+ barrier();
+
+ TCP_SKB_CB(skb)->seq = ntohl(th->seq);
+ TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
+ skb->len - th->doff * 4);
+ TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
+ TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
+ TCP_SKB_CB(skb)->tcp_tw_isn = 0;
+ TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
+ TCP_SKB_CB(skb)->sacked = 0;
+ TCP_SKB_CB(skb)->has_rxtstamp =
+ skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
+}
+
/*
* From tcp_input.c
*/
@@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
- /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
- * barrier() makes sure compiler wont play fool^Waliasing games.
- */
- memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
- sizeof(struct inet_skb_parm));
- barrier();
-
- TCP_SKB_CB(skb)->seq = ntohl(th->seq);
- TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff * 4);
- TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
- TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
- TCP_SKB_CB(skb)->tcp_tw_isn = 0;
- TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
- TCP_SKB_CB(skb)->sacked = 0;
- TCP_SKB_CB(skb)->has_rxtstamp =
- skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
-
lookup:
sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
th->dest, sdif, &refcounted);
@@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
sock_hold(sk);
refcounted = true;
nsk = NULL;
- if (!tcp_filter(sk, skb))
+ if (!tcp_filter(sk, skb)) {
+ th = (const struct tcphdr *)skb->data;
+ iph = ip_hdr(skb);
+ tcp_v4_fill_cb(skb, iph, th);
nsk = tcp_check_req(sk, skb, req, false);
+ }
if (!nsk) {
reqsk_put(req);
goto discard_and_relse;
}
if (nsk == sk) {
reqsk_put(req);
+ tcp_v4_restore_cb(skb);
} else if (tcp_child_process(sk, nsk, skb)) {
tcp_v4_send_reset(nsk, skb);
goto discard_and_relse;
@@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_and_relse;
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
+ tcp_v4_fill_cb(skb, iph, th);
skb->dev = NULL;
@@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
+ tcp_v4_fill_cb(skb, iph, th);
+
if (tcp_checksum_complete(skb)) {
csum_error:
__TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
@@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_it;
}
+ tcp_v4_fill_cb(skb, iph, th);
+
if (tcp_checksum_complete(skb)) {
inet_twsk_put(inet_twsk(sk));
goto csum_error;
@@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
if (sk2) {
inet_twsk_deschedule_put(inet_twsk(sk));
sk = sk2;
+ tcp_v4_restore_cb(skb);
refcounted = false;
goto process;
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
struct sock *nsk;
sk = req->rsk_listener;
- tcp_v6_fill_cb(skb, hdr, th);
if (tcp_v6_inbound_md5_hash(sk, skb)) {
sk_drops_add(sk, skb);
reqsk_put(req);
@@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
sock_hold(sk);
refcounted = true;
nsk = NULL;
- if (!tcp_filter(sk, skb))
+ if (!tcp_filter(sk, skb)) {
+ th = (const struct tcphdr *)skb->data;
+ hdr = ipv6_hdr(skb);
+ tcp_v6_fill_cb(skb, hdr, th);
nsk = tcp_check_req(sk, skb, req, false);
+ }
if (!nsk) {
reqsk_put(req);
goto discard_and_relse;
@@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_and_relse;
- tcp_v6_fill_cb(skb, hdr, th);
-
if (tcp_v6_inbound_md5_hash(sk, skb))
goto discard_and_relse;
@@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
goto discard_and_relse;
th = (const struct tcphdr *)skb->data;
hdr = ipv6_hdr(skb);
+ tcp_v6_fill_cb(skb, hdr, th);
skb->dev = NULL;
@@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
tcp_v6_timewait_ack(sk, skb);
break;
case TCP_TW_RST:
- tcp_v6_restore_cb(skb);
tcp_v6_send_reset(sk, skb);
inet_twsk_deschedule_put(inet_twsk(sk));
goto discard_it;
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 10:50 ` Eric Dumazet
@ 2017-11-30 12:47 ` Paul Moore
-1 siblings, 0 replies; 38+ messages in thread
From: Paul Moore @ 2017-11-30 12:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Casey Schaufler, James Morris, netdev, Stephen Smalley, selinux, LSM
On Thu, Nov 30, 2017 at 5:50 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>> On 11/29/2017 4:31 PM, James Morris wrote:
>> > On Wed, 29 Nov 2017, Casey Schaufler wrote:
>> >
>> > > I see that there is a proposed fix later in the thread, but I
>> > > don't see
>> > > the patch. Could you send it to me, so I can try it on my
>> > > problem?
>> >
>> > Forwarded off-list.
>>
>> The patch does fix the problem I was seeing in Smack.
>
> Can you guys test the following more complete patch ?
>
> It should cover IPv4 and IPv6, and also the corner cases.
>
> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
> 42844/ that I spotted while cooking this patch )
Building a test kernel now, although it make take me a few hours to
test it due to some commitments this morning.
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(tcp_filter);
>
> +static void tcp_v4_restore_cb(struct sk_buff *skb)
> +{
> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
> + sizeof(struct inet_skb_parm));
> +}
> +
> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> + const struct tcphdr *th)
> +{
> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> + * barrier() makes sure compiler wont play fool^Waliasing games.
> + */
> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> + sizeof(struct inet_skb_parm));
> + barrier();
> +
> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> + skb->len - th->doff * 4);
> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> + TCP_SKB_CB(skb)->sacked = 0;
> + TCP_SKB_CB(skb)->has_rxtstamp =
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> +}
> +
> /*
> * From tcp_input.c
> */
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> }
> if (nsk == sk) {
> reqsk_put(req);
> + tcp_v4_restore_cb(skb);
> } else if (tcp_child_process(sk, nsk, skb)) {
> tcp_v4_send_reset(nsk, skb);
> goto discard_and_relse;
> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
>
> skb->dev = NULL;
>
> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
> goto discard_it;
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> csum_error:
> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_it;
> }
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> inet_twsk_put(inet_twsk(sk));
> goto csum_error;
> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (sk2) {
> inet_twsk_deschedule_put(inet_twsk(sk));
> sk = sk2;
> + tcp_v4_restore_cb(skb);
> refcounted = false;
> goto process;
> }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> struct sock *nsk;
>
> sk = req->rsk_listener;
> - tcp_v6_fill_cb(skb, hdr, th);
> if (tcp_v6_inbound_md5_hash(sk, skb)) {
> sk_drops_add(sk, skb);
> reqsk_put(req);
> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
>
> - tcp_v6_fill_cb(skb, hdr, th);
> -
> if (tcp_v6_inbound_md5_hash(sk, skb))
> goto discard_and_relse;
>
> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
>
> skb->dev = NULL;
>
> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> tcp_v6_timewait_ack(sk, skb);
> break;
> case TCP_TW_RST:
> - tcp_v6_restore_cb(skb);
> tcp_v6_send_reset(sk, skb);
> inet_twsk_deschedule_put(inet_twsk(sk));
> goto discard_it;
>
>
>
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 12:47 ` Paul Moore
0 siblings, 0 replies; 38+ messages in thread
From: Paul Moore @ 2017-11-30 12:47 UTC (permalink / raw)
To: linux-security-module
On Thu, Nov 30, 2017 at 5:50 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>> On 11/29/2017 4:31 PM, James Morris wrote:
>> > On Wed, 29 Nov 2017, Casey Schaufler wrote:
>> >
>> > > I see that there is a proposed fix later in the thread, but I
>> > > don't see
>> > > the patch. Could you send it to me, so I can try it on my
>> > > problem?
>> >
>> > Forwarded off-list.
>>
>> The patch does fix the problem I was seeing in Smack.
>
> Can you guys test the following more complete patch ?
>
> It should cover IPv4 and IPv6, and also the corner cases.
>
> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
> 42844/ that I spotted while cooking this patch )
Building a test kernel now, although it make take me a few hours to
test it due to some commitments this morning.
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(tcp_filter);
>
> +static void tcp_v4_restore_cb(struct sk_buff *skb)
> +{
> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
> + sizeof(struct inet_skb_parm));
> +}
> +
> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> + const struct tcphdr *th)
> +{
> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> + * barrier() makes sure compiler wont play fool^Waliasing games.
> + */
> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> + sizeof(struct inet_skb_parm));
> + barrier();
> +
> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> + skb->len - th->doff * 4);
> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> + TCP_SKB_CB(skb)->sacked = 0;
> + TCP_SKB_CB(skb)->has_rxtstamp =
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> +}
> +
> /*
> * From tcp_input.c
> */
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> }
> if (nsk == sk) {
> reqsk_put(req);
> + tcp_v4_restore_cb(skb);
> } else if (tcp_child_process(sk, nsk, skb)) {
> tcp_v4_send_reset(nsk, skb);
> goto discard_and_relse;
> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
>
> skb->dev = NULL;
>
> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
> goto discard_it;
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> csum_error:
> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_it;
> }
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> inet_twsk_put(inet_twsk(sk));
> goto csum_error;
> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (sk2) {
> inet_twsk_deschedule_put(inet_twsk(sk));
> sk = sk2;
> + tcp_v4_restore_cb(skb);
> refcounted = false;
> goto process;
> }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> struct sock *nsk;
>
> sk = req->rsk_listener;
> - tcp_v6_fill_cb(skb, hdr, th);
> if (tcp_v6_inbound_md5_hash(sk, skb)) {
> sk_drops_add(sk, skb);
> reqsk_put(req);
> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
>
> - tcp_v6_fill_cb(skb, hdr, th);
> -
> if (tcp_v6_inbound_md5_hash(sk, skb))
> goto discard_and_relse;
>
> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
>
> skb->dev = NULL;
>
> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> tcp_v6_timewait_ack(sk, skb);
> break;
> case TCP_TW_RST:
> - tcp_v6_restore_cb(skb);
> tcp_v6_send_reset(sk, skb);
> inet_twsk_deschedule_put(inet_twsk(sk));
> goto discard_it;
>
>
>
--
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 10:50 ` Eric Dumazet
@ 2017-11-30 14:33 ` Casey Schaufler
-1 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 14:33 UTC (permalink / raw)
To: Eric Dumazet, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On 11/30/2017 2:50 AM, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>> On 11/29/2017 4:31 PM, James Morris wrote:
>>> On Wed, 29 Nov 2017, Casey Schaufler wrote:
>>>
>>>> I see that there is a proposed fix later in the thread, but I
>>>> don't see
>>>> the patch. Could you send it to me, so I can try it on my
>>>> problem?
>>> Forwarded off-list.
>> The patch does fix the problem I was seeing in Smack.
> Can you guys test the following more complete patch ?
Building now. I should have results soon.
>
> It should cover IPv4 and IPv6, and also the corner cases.
>
> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
> 42844/ that I spotted while cooking this patch )
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(tcp_filter);
>
> +static void tcp_v4_restore_cb(struct sk_buff *skb)
> +{
> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
> + sizeof(struct inet_skb_parm));
> +}
> +
> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> + const struct tcphdr *th)
> +{
> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> + * barrier() makes sure compiler wont play fool^Waliasing games.
> + */
> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> + sizeof(struct inet_skb_parm));
> + barrier();
> +
> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> + skb->len - th->doff * 4);
> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> + TCP_SKB_CB(skb)->sacked = 0;
> + TCP_SKB_CB(skb)->has_rxtstamp =
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> +}
> +
> /*
> * From tcp_input.c
> */
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> }
> if (nsk == sk) {
> reqsk_put(req);
> + tcp_v4_restore_cb(skb);
> } else if (tcp_child_process(sk, nsk, skb)) {
> tcp_v4_send_reset(nsk, skb);
> goto discard_and_relse;
> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
>
> skb->dev = NULL;
>
> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
> goto discard_it;
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> csum_error:
> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_it;
> }
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> inet_twsk_put(inet_twsk(sk));
> goto csum_error;
> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (sk2) {
> inet_twsk_deschedule_put(inet_twsk(sk));
> sk = sk2;
> + tcp_v4_restore_cb(skb);
> refcounted = false;
> goto process;
> }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> struct sock *nsk;
>
> sk = req->rsk_listener;
> - tcp_v6_fill_cb(skb, hdr, th);
> if (tcp_v6_inbound_md5_hash(sk, skb)) {
> sk_drops_add(sk, skb);
> reqsk_put(req);
> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
>
> - tcp_v6_fill_cb(skb, hdr, th);
> -
> if (tcp_v6_inbound_md5_hash(sk, skb))
> goto discard_and_relse;
>
> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
>
> skb->dev = NULL;
>
> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> tcp_v6_timewait_ack(sk, skb);
> break;
> case TCP_TW_RST:
> - tcp_v6_restore_cb(skb);
> tcp_v6_send_reset(sk, skb);
> inet_twsk_deschedule_put(inet_twsk(sk));
> goto discard_it;
>
>
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 14:33 ` Casey Schaufler
0 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 14:33 UTC (permalink / raw)
To: linux-security-module
On 11/30/2017 2:50 AM, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>> On 11/29/2017 4:31 PM, James Morris wrote:
>>> On Wed, 29 Nov 2017, Casey Schaufler wrote:
>>>
>>>> I see that there is a proposed fix later in the thread, but I
>>>> don't see
>>>> the patch. Could you send it to me, so I can try it on my
>>>> problem?
>>> Forwarded off-list.
>> The patch does fix the problem I was seeing in Smack.
> Can you guys test the following more complete patch ?
Building now. I should have results soon.
>
> It should cover IPv4 and IPv6, and also the corner cases.
>
> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
> 42844/ that I spotted while cooking this patch )
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(tcp_filter);
>
> +static void tcp_v4_restore_cb(struct sk_buff *skb)
> +{
> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
> + sizeof(struct inet_skb_parm));
> +}
> +
> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> + const struct tcphdr *th)
> +{
> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> + * barrier() makes sure compiler wont play fool^Waliasing games.
> + */
> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> + sizeof(struct inet_skb_parm));
> + barrier();
> +
> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> + skb->len - th->doff * 4);
> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> + TCP_SKB_CB(skb)->sacked = 0;
> + TCP_SKB_CB(skb)->has_rxtstamp =
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> +}
> +
> /*
> * From tcp_input.c
> */
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> }
> if (nsk == sk) {
> reqsk_put(req);
> + tcp_v4_restore_cb(skb);
> } else if (tcp_child_process(sk, nsk, skb)) {
> tcp_v4_send_reset(nsk, skb);
> goto discard_and_relse;
> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
>
> skb->dev = NULL;
>
> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
> goto discard_it;
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> csum_error:
> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_it;
> }
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> inet_twsk_put(inet_twsk(sk));
> goto csum_error;
> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (sk2) {
> inet_twsk_deschedule_put(inet_twsk(sk));
> sk = sk2;
> + tcp_v4_restore_cb(skb);
> refcounted = false;
> goto process;
> }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> struct sock *nsk;
>
> sk = req->rsk_listener;
> - tcp_v6_fill_cb(skb, hdr, th);
> if (tcp_v6_inbound_md5_hash(sk, skb)) {
> sk_drops_add(sk, skb);
> reqsk_put(req);
> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
>
> - tcp_v6_fill_cb(skb, hdr, th);
> -
> if (tcp_v6_inbound_md5_hash(sk, skb))
> goto discard_and_relse;
>
> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
>
> skb->dev = NULL;
>
> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> tcp_v6_timewait_ack(sk, skb);
> break;
> case TCP_TW_RST:
> - tcp_v6_restore_cb(skb);
> tcp_v6_send_reset(sk, skb);
> inet_twsk_deschedule_put(inet_twsk(sk));
> goto discard_it;
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 10:50 ` Eric Dumazet
@ 2017-11-30 15:11 ` Casey Schaufler
-1 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 15:11 UTC (permalink / raw)
To: Eric Dumazet, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On 11/30/2017 2:50 AM, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>> On 11/29/2017 4:31 PM, James Morris wrote:
>>> On Wed, 29 Nov 2017, Casey Schaufler wrote:
>>>
>>>> I see that there is a proposed fix later in the thread, but I
>>>> don't see
>>>> the patch. Could you send it to me, so I can try it on my
>>>> problem?
>>> Forwarded off-list.
>> The patch does fix the problem I was seeing in Smack.
> Can you guys test the following more complete patch ?
My tests are passing. Thank you.
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
>
> It should cover IPv4 and IPv6, and also the corner cases.
>
> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
> 42844/ that I spotted while cooking this patch )
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(tcp_filter);
>
> +static void tcp_v4_restore_cb(struct sk_buff *skb)
> +{
> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
> + sizeof(struct inet_skb_parm));
> +}
> +
> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> + const struct tcphdr *th)
> +{
> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> + * barrier() makes sure compiler wont play fool^Waliasing games.
> + */
> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> + sizeof(struct inet_skb_parm));
> + barrier();
> +
> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> + skb->len - th->doff * 4);
> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> + TCP_SKB_CB(skb)->sacked = 0;
> + TCP_SKB_CB(skb)->has_rxtstamp =
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> +}
> +
> /*
> * From tcp_input.c
> */
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> }
> if (nsk == sk) {
> reqsk_put(req);
> + tcp_v4_restore_cb(skb);
> } else if (tcp_child_process(sk, nsk, skb)) {
> tcp_v4_send_reset(nsk, skb);
> goto discard_and_relse;
> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
>
> skb->dev = NULL;
>
> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
> goto discard_it;
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> csum_error:
> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_it;
> }
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> inet_twsk_put(inet_twsk(sk));
> goto csum_error;
> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (sk2) {
> inet_twsk_deschedule_put(inet_twsk(sk));
> sk = sk2;
> + tcp_v4_restore_cb(skb);
> refcounted = false;
> goto process;
> }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> struct sock *nsk;
>
> sk = req->rsk_listener;
> - tcp_v6_fill_cb(skb, hdr, th);
> if (tcp_v6_inbound_md5_hash(sk, skb)) {
> sk_drops_add(sk, skb);
> reqsk_put(req);
> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
>
> - tcp_v6_fill_cb(skb, hdr, th);
> -
> if (tcp_v6_inbound_md5_hash(sk, skb))
> goto discard_and_relse;
>
> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
>
> skb->dev = NULL;
>
> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> tcp_v6_timewait_ack(sk, skb);
> break;
> case TCP_TW_RST:
> - tcp_v6_restore_cb(skb);
> tcp_v6_send_reset(sk, skb);
> inet_twsk_deschedule_put(inet_twsk(sk));
> goto discard_it;
>
>
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 15:11 ` Casey Schaufler
0 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 15:11 UTC (permalink / raw)
To: linux-security-module
On 11/30/2017 2:50 AM, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>> On 11/29/2017 4:31 PM, James Morris wrote:
>>> On Wed, 29 Nov 2017, Casey Schaufler wrote:
>>>
>>>> I see that there is a proposed fix later in the thread, but I
>>>> don't see
>>>> the patch. Could you send it to me, so I can try it on my
>>>> problem?
>>> Forwarded off-list.
>> The patch does fix the problem I was seeing in Smack.
> Can you guys test the following more complete patch ?
My tests are passing. Thank you.
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
>
> It should cover IPv4 and IPv6, and also the corner cases.
>
> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
> 42844/ that I spotted while cooking this patch )
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(tcp_filter);
>
> +static void tcp_v4_restore_cb(struct sk_buff *skb)
> +{
> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
> + sizeof(struct inet_skb_parm));
> +}
> +
> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> + const struct tcphdr *th)
> +{
> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> + * barrier() makes sure compiler wont play fool^Waliasing games.
> + */
> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> + sizeof(struct inet_skb_parm));
> + barrier();
> +
> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> + skb->len - th->doff * 4);
> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> + TCP_SKB_CB(skb)->sacked = 0;
> + TCP_SKB_CB(skb)->has_rxtstamp =
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> +}
> +
> /*
> * From tcp_input.c
> */
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> }
> if (nsk == sk) {
> reqsk_put(req);
> + tcp_v4_restore_cb(skb);
> } else if (tcp_child_process(sk, nsk, skb)) {
> tcp_v4_send_reset(nsk, skb);
> goto discard_and_relse;
> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> + tcp_v4_fill_cb(skb, iph, th);
>
> skb->dev = NULL;
>
> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
> goto discard_it;
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> csum_error:
> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
> goto discard_it;
> }
>
> + tcp_v4_fill_cb(skb, iph, th);
> +
> if (tcp_checksum_complete(skb)) {
> inet_twsk_put(inet_twsk(sk));
> goto csum_error;
> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> if (sk2) {
> inet_twsk_deschedule_put(inet_twsk(sk));
> sk = sk2;
> + tcp_v4_restore_cb(skb);
> refcounted = false;
> goto process;
> }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> struct sock *nsk;
>
> sk = req->rsk_listener;
> - tcp_v6_fill_cb(skb, hdr, th);
> if (tcp_v6_inbound_md5_hash(sk, skb)) {
> sk_drops_add(sk, skb);
> reqsk_put(req);
> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> sock_hold(sk);
> refcounted = true;
> nsk = NULL;
> - if (!tcp_filter(sk, skb))
> + if (!tcp_filter(sk, skb)) {
> + th = (const struct tcphdr *)skb->data;
> + hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
> nsk = tcp_check_req(sk, skb, req, false);
> + }
> if (!nsk) {
> reqsk_put(req);
> goto discard_and_relse;
> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
>
> - tcp_v6_fill_cb(skb, hdr, th);
> -
> if (tcp_v6_inbound_md5_hash(sk, skb))
> goto discard_and_relse;
>
> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> goto discard_and_relse;
> th = (const struct tcphdr *)skb->data;
> hdr = ipv6_hdr(skb);
> + tcp_v6_fill_cb(skb, hdr, th);
>
> skb->dev = NULL;
>
> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
> tcp_v6_timewait_ack(sk, skb);
> break;
> case TCP_TW_RST:
> - tcp_v6_restore_cb(skb);
> tcp_v6_send_reset(sk, skb);
> inet_twsk_deschedule_put(inet_twsk(sk));
> goto discard_it;
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 10:50 ` Eric Dumazet
@ 2017-11-30 15:44 ` David Ahern
-1 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2017-11-30 15:44 UTC (permalink / raw)
To: Eric Dumazet, Casey Schaufler, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On 11/30/17 3:50 AM, Eric Dumazet wrote:
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
I believe moving the above is going to affect lookups with VRF. Let me
take a look before this gets committed.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 15:44 ` David Ahern
0 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2017-11-30 15:44 UTC (permalink / raw)
To: linux-security-module
On 11/30/17 3:50 AM, Eric Dumazet wrote:
> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>
> th = (const struct tcphdr *)skb->data;
> iph = ip_hdr(skb);
> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
> - * barrier() makes sure compiler wont play fool^Waliasing games.
> - */
> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> - sizeof(struct inet_skb_parm));
> - barrier();
> -
> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
> - skb->len - th->doff * 4);
> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> - TCP_SKB_CB(skb)->sacked = 0;
> - TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> -
> lookup:
> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
> th->dest, sdif, &refcounted);
I believe moving the above is going to affect lookups with VRF. Let me
take a look before this gets committed.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 12:47 ` Paul Moore
@ 2017-11-30 16:57 ` Paul Moore
-1 siblings, 0 replies; 38+ messages in thread
From: Paul Moore @ 2017-11-30 16:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: Casey Schaufler, James Morris, netdev, Stephen Smalley, selinux, LSM
On Thu, Nov 30, 2017 at 7:47 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Nov 30, 2017 at 5:50 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>>> On 11/29/2017 4:31 PM, James Morris wrote:
>>> > On Wed, 29 Nov 2017, Casey Schaufler wrote:
>>> >
>>> > > I see that there is a proposed fix later in the thread, but I
>>> > > don't see
>>> > > the patch. Could you send it to me, so I can try it on my
>>> > > problem?
>>> >
>>> > Forwarded off-list.
>>>
>>> The patch does fix the problem I was seeing in Smack.
>>
>> Can you guys test the following more complete patch ?
>>
>> It should cover IPv4 and IPv6, and also the corner cases.
>>
>> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
>> 42844/ that I spotted while cooking this patch )
>
> Building a test kernel now, although it make take me a few hours to
> test it due to some commitments this morning.
I just realized I forgot to enable KASAN in the build, but I can
verify that the patch doesn't break anything in the selinux-testsuite.
Tested-by: Paul Moore <paul@paul-moore.com>
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
>> }
>> EXPORT_SYMBOL(tcp_filter);
>>
>> +static void tcp_v4_restore_cb(struct sk_buff *skb)
>> +{
>> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
>> + sizeof(struct inet_skb_parm));
>> +}
>> +
>> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
>> + const struct tcphdr *th)
>> +{
>> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
>> + * barrier() makes sure compiler wont play fool^Waliasing games.
>> + */
>> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>> + sizeof(struct inet_skb_parm));
>> + barrier();
>> +
>> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
>> + skb->len - th->doff * 4);
>> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>> + TCP_SKB_CB(skb)->sacked = 0;
>> + TCP_SKB_CB(skb)->has_rxtstamp =
>> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
>> +}
>> +
>> /*
>> * From tcp_input.c
>> */
>> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>
>> th = (const struct tcphdr *)skb->data;
>> iph = ip_hdr(skb);
>> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
>> - * barrier() makes sure compiler wont play fool^Waliasing games.
>> - */
>> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>> - sizeof(struct inet_skb_parm));
>> - barrier();
>> -
>> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
>> - skb->len - th->doff * 4);
>> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>> - TCP_SKB_CB(skb)->sacked = 0;
>> - TCP_SKB_CB(skb)->has_rxtstamp =
>> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
>> -
>> lookup:
>> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
>> th->dest, sdif, &refcounted);
>> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> sock_hold(sk);
>> refcounted = true;
>> nsk = NULL;
>> - if (!tcp_filter(sk, skb))
>> + if (!tcp_filter(sk, skb)) {
>> + th = (const struct tcphdr *)skb->data;
>> + iph = ip_hdr(skb);
>> + tcp_v4_fill_cb(skb, iph, th);
>> nsk = tcp_check_req(sk, skb, req, false);
>> + }
>> if (!nsk) {
>> reqsk_put(req);
>> goto discard_and_relse;
>> }
>> if (nsk == sk) {
>> reqsk_put(req);
>> + tcp_v4_restore_cb(skb);
>> } else if (tcp_child_process(sk, nsk, skb)) {
>> tcp_v4_send_reset(nsk, skb);
>> goto discard_and_relse;
>> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> goto discard_and_relse;
>> th = (const struct tcphdr *)skb->data;
>> iph = ip_hdr(skb);
>> + tcp_v4_fill_cb(skb, iph, th);
>>
>> skb->dev = NULL;
>>
>> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
>> goto discard_it;
>>
>> + tcp_v4_fill_cb(skb, iph, th);
>> +
>> if (tcp_checksum_complete(skb)) {
>> csum_error:
>> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
>> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> goto discard_it;
>> }
>>
>> + tcp_v4_fill_cb(skb, iph, th);
>> +
>> if (tcp_checksum_complete(skb)) {
>> inet_twsk_put(inet_twsk(sk));
>> goto csum_error;
>> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> if (sk2) {
>> inet_twsk_deschedule_put(inet_twsk(sk));
>> sk = sk2;
>> + tcp_v4_restore_cb(skb);
>> refcounted = false;
>> goto process;
>> }
>> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
>> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
>> --- a/net/ipv6/tcp_ipv6.c
>> +++ b/net/ipv6/tcp_ipv6.c
>> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> struct sock *nsk;
>>
>> sk = req->rsk_listener;
>> - tcp_v6_fill_cb(skb, hdr, th);
>> if (tcp_v6_inbound_md5_hash(sk, skb)) {
>> sk_drops_add(sk, skb);
>> reqsk_put(req);
>> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> sock_hold(sk);
>> refcounted = true;
>> nsk = NULL;
>> - if (!tcp_filter(sk, skb))
>> + if (!tcp_filter(sk, skb)) {
>> + th = (const struct tcphdr *)skb->data;
>> + hdr = ipv6_hdr(skb);
>> + tcp_v6_fill_cb(skb, hdr, th);
>> nsk = tcp_check_req(sk, skb, req, false);
>> + }
>> if (!nsk) {
>> reqsk_put(req);
>> goto discard_and_relse;
>> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
>> goto discard_and_relse;
>>
>> - tcp_v6_fill_cb(skb, hdr, th);
>> -
>> if (tcp_v6_inbound_md5_hash(sk, skb))
>> goto discard_and_relse;
>>
>> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> goto discard_and_relse;
>> th = (const struct tcphdr *)skb->data;
>> hdr = ipv6_hdr(skb);
>> + tcp_v6_fill_cb(skb, hdr, th);
>>
>> skb->dev = NULL;
>>
>> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> tcp_v6_timewait_ack(sk, skb);
>> break;
>> case TCP_TW_RST:
>> - tcp_v6_restore_cb(skb);
>> tcp_v6_send_reset(sk, skb);
>> inet_twsk_deschedule_put(inet_twsk(sk));
>> goto discard_it;
>>
>>
>>
>
>
>
> --
> paul moore
> www.paul-moore.com
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 16:57 ` Paul Moore
0 siblings, 0 replies; 38+ messages in thread
From: Paul Moore @ 2017-11-30 16:57 UTC (permalink / raw)
To: linux-security-module
On Thu, Nov 30, 2017 at 7:47 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Nov 30, 2017 at 5:50 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
>>> On 11/29/2017 4:31 PM, James Morris wrote:
>>> > On Wed, 29 Nov 2017, Casey Schaufler wrote:
>>> >
>>> > > I see that there is a proposed fix later in the thread, but I
>>> > > don't see
>>> > > the patch. Could you send it to me, so I can try it on my
>>> > > problem?
>>> >
>>> > Forwarded off-list.
>>>
>>> The patch does fix the problem I was seeing in Smack.
>>
>> Can you guys test the following more complete patch ?
>>
>> It should cover IPv4 and IPv6, and also the corner cases.
>>
>> ( Note that I squashed ipv6 fix in https://patchwork.ozlabs.org/patch/8
>> 42844/ that I spotted while cooking this patch )
>
> Building a test kernel now, although it make take me a few hours to
> test it due to some commitments this morning.
I just realized I forgot to enable KASAN in the build, but I can
verify that the patch doesn't break anything in the selinux-testsuite.
Tested-by: Paul Moore <paul@paul-moore.com>
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index c6bc0c4d19c624888b0d0b5a4246c7183edf63f5..77ea45da0fe9c746907a312989658af3ad3b198d 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -1591,6 +1591,34 @@ int tcp_filter(struct sock *sk, struct sk_buff *skb)
>> }
>> EXPORT_SYMBOL(tcp_filter);
>>
>> +static void tcp_v4_restore_cb(struct sk_buff *skb)
>> +{
>> + memmove(IPCB(skb), &TCP_SKB_CB(skb)->header.h4,
>> + sizeof(struct inet_skb_parm));
>> +}
>> +
>> +static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
>> + const struct tcphdr *th)
>> +{
>> + /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
>> + * barrier() makes sure compiler wont play fool^Waliasing games.
>> + */
>> + memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>> + sizeof(struct inet_skb_parm));
>> + barrier();
>> +
>> + TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>> + TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
>> + skb->len - th->doff * 4);
>> + TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>> + TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>> + TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>> + TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>> + TCP_SKB_CB(skb)->sacked = 0;
>> + TCP_SKB_CB(skb)->has_rxtstamp =
>> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
>> +}
>> +
>> /*
>> * From tcp_input.c
>> */
>> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>
>> th = (const struct tcphdr *)skb->data;
>> iph = ip_hdr(skb);
>> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
>> - * barrier() makes sure compiler wont play fool^Waliasing games.
>> - */
>> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>> - sizeof(struct inet_skb_parm));
>> - barrier();
>> -
>> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
>> - skb->len - th->doff * 4);
>> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>> - TCP_SKB_CB(skb)->sacked = 0;
>> - TCP_SKB_CB(skb)->has_rxtstamp =
>> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
>> -
>> lookup:
>> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
>> th->dest, sdif, &refcounted);
>> @@ -1679,14 +1689,19 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> sock_hold(sk);
>> refcounted = true;
>> nsk = NULL;
>> - if (!tcp_filter(sk, skb))
>> + if (!tcp_filter(sk, skb)) {
>> + th = (const struct tcphdr *)skb->data;
>> + iph = ip_hdr(skb);
>> + tcp_v4_fill_cb(skb, iph, th);
>> nsk = tcp_check_req(sk, skb, req, false);
>> + }
>> if (!nsk) {
>> reqsk_put(req);
>> goto discard_and_relse;
>> }
>> if (nsk == sk) {
>> reqsk_put(req);
>> + tcp_v4_restore_cb(skb);
>> } else if (tcp_child_process(sk, nsk, skb)) {
>> tcp_v4_send_reset(nsk, skb);
>> goto discard_and_relse;
>> @@ -1712,6 +1727,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> goto discard_and_relse;
>> th = (const struct tcphdr *)skb->data;
>> iph = ip_hdr(skb);
>> + tcp_v4_fill_cb(skb, iph, th);
>>
>> skb->dev = NULL;
>>
>> @@ -1742,6 +1758,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
>> goto discard_it;
>>
>> + tcp_v4_fill_cb(skb, iph, th);
>> +
>> if (tcp_checksum_complete(skb)) {
>> csum_error:
>> __TCP_INC_STATS(net, TCP_MIB_CSUMERRORS);
>> @@ -1768,6 +1786,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> goto discard_it;
>> }
>>
>> + tcp_v4_fill_cb(skb, iph, th);
>> +
>> if (tcp_checksum_complete(skb)) {
>> inet_twsk_put(inet_twsk(sk));
>> goto csum_error;
>> @@ -1784,6 +1804,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
>> if (sk2) {
>> inet_twsk_deschedule_put(inet_twsk(sk));
>> sk = sk2;
>> + tcp_v4_restore_cb(skb);
>> refcounted = false;
>> goto process;
>> }
>> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
>> index 6bb98c93edfe2ed2f16fe5229605f8108cfc7f9a..1f04ec0e4a7aa2c11b8ee27cbdd4067b5bcf32e5 100644
>> --- a/net/ipv6/tcp_ipv6.c
>> +++ b/net/ipv6/tcp_ipv6.c
>> @@ -1454,7 +1454,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> struct sock *nsk;
>>
>> sk = req->rsk_listener;
>> - tcp_v6_fill_cb(skb, hdr, th);
>> if (tcp_v6_inbound_md5_hash(sk, skb)) {
>> sk_drops_add(sk, skb);
>> reqsk_put(req);
>> @@ -1467,8 +1466,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> sock_hold(sk);
>> refcounted = true;
>> nsk = NULL;
>> - if (!tcp_filter(sk, skb))
>> + if (!tcp_filter(sk, skb)) {
>> + th = (const struct tcphdr *)skb->data;
>> + hdr = ipv6_hdr(skb);
>> + tcp_v6_fill_cb(skb, hdr, th);
>> nsk = tcp_check_req(sk, skb, req, false);
>> + }
>> if (!nsk) {
>> reqsk_put(req);
>> goto discard_and_relse;
>> @@ -1492,8 +1495,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
>> goto discard_and_relse;
>>
>> - tcp_v6_fill_cb(skb, hdr, th);
>> -
>> if (tcp_v6_inbound_md5_hash(sk, skb))
>> goto discard_and_relse;
>>
>> @@ -1501,6 +1502,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> goto discard_and_relse;
>> th = (const struct tcphdr *)skb->data;
>> hdr = ipv6_hdr(skb);
>> + tcp_v6_fill_cb(skb, hdr, th);
>>
>> skb->dev = NULL;
>>
>> @@ -1590,7 +1592,6 @@ static int tcp_v6_rcv(struct sk_buff *skb)
>> tcp_v6_timewait_ack(sk, skb);
>> break;
>> case TCP_TW_RST:
>> - tcp_v6_restore_cb(skb);
>> tcp_v6_send_reset(sk, skb);
>> inet_twsk_deschedule_put(inet_twsk(sk));
>> goto discard_it;
>>
>>
>>
>
>
>
> --
> paul moore
> www.paul-moore.com
--
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 15:44 ` David Ahern
@ 2017-11-30 17:30 ` David Ahern
-1 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2017-11-30 17:30 UTC (permalink / raw)
To: Eric Dumazet, Casey Schaufler, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On 11/30/17 8:44 AM, David Ahern wrote:
> On 11/30/17 3:50 AM, Eric Dumazet wrote:
>> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>
>> th = (const struct tcphdr *)skb->data;
>> iph = ip_hdr(skb);
>> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
>> - * barrier() makes sure compiler wont play fool^Waliasing games.
>> - */
>> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>> - sizeof(struct inet_skb_parm));
>> - barrier();
>> -
>> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
>> - skb->len - th->doff * 4);
>> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>> - TCP_SKB_CB(skb)->sacked = 0;
>> - TCP_SKB_CB(skb)->has_rxtstamp =
>> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
>> -
>> lookup:
>> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
>> th->dest, sdif, &refcounted);
>
> I believe moving the above is going to affect lookups with VRF. Let me
> take a look before this gets committed.
>
Eric:
Can you add this to the patch? Fixes socket lookups with VRF which
stashes a flag in the cb.
Thanks,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4e09398009c1..6c020015d556 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct net
*net, struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
- skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
+ skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
return true;
#endif
return false;
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 17:30 ` David Ahern
0 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2017-11-30 17:30 UTC (permalink / raw)
To: linux-security-module
On 11/30/17 8:44 AM, David Ahern wrote:
> On 11/30/17 3:50 AM, Eric Dumazet wrote:
>> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>
>> th = (const struct tcphdr *)skb->data;
>> iph = ip_hdr(skb);
>> - /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
>> - * barrier() makes sure compiler wont play fool^Waliasing games.
>> - */
>> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>> - sizeof(struct inet_skb_parm));
>> - barrier();
>> -
>> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
>> - skb->len - th->doff * 4);
>> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>> - TCP_SKB_CB(skb)->sacked = 0;
>> - TCP_SKB_CB(skb)->has_rxtstamp =
>> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
>> -
>> lookup:
>> sk = __inet_lookup_skb(&tcp_hashinfo, skb, __tcp_hdrlen(th), th->source,
>> th->dest, sdif, &refcounted);
>
> I believe moving the above is going to affect lookups with VRF. Let me
> take a look before this gets committed.
>
Eric:
Can you add this to the patch? Fixes socket lookups with VRF which
stashes a flag in the cb.
Thanks,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4e09398009c1..6c020015d556 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct net
*net, struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
- skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
+ skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
return true;
#endif
return false;
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 17:30 ` David Ahern
@ 2017-11-30 17:57 ` Eric Dumazet
-1 siblings, 0 replies; 38+ messages in thread
From: Eric Dumazet @ 2017-11-30 17:57 UTC (permalink / raw)
To: David Ahern, Casey Schaufler, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On Thu, 2017-11-30 at 10:30 -0700, David Ahern wrote:
> On 11/30/17 8:44 AM, David Ahern wrote:
> > On 11/30/17 3:50 AM, Eric Dumazet wrote:
> > > @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
> > >
> > > th = (const struct tcphdr *)skb->data;
> > > iph = ip_hdr(skb);
> > > - /* This is tricky : We move IPCB at its correct location
> > > into TCP_SKB_CB()
> > > - * barrier() makes sure compiler wont play
> > > fool^Waliasing games.
> > > - */
> > > - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> > > - sizeof(struct inet_skb_parm));
> > > - barrier();
> > > -
> > > - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> > > - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th-
> > > >syn + th->fin +
> > > - skb->len - th->doff * 4);
> > > - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> > > - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> > > - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> > > - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> > > - TCP_SKB_CB(skb)->sacked = 0;
> > > - TCP_SKB_CB(skb)->has_rxtstamp =
> > > - skb->tstamp || skb_hwtstamps(skb)-
> > > >hwtstamp;
> > > -
> > > lookup:
> > > sk = __inet_lookup_skb(&tcp_hashinfo, skb,
> > > __tcp_hdrlen(th), th->source,
> > > th->dest, sdif, &refcounted);
> >
> > I believe moving the above is going to affect lookups with VRF. Let
> > me
> > take a look before this gets committed.
> >
>
> Eric:
>
> Can you add this to the patch? Fixes socket lookups with VRF which
> stashes a flag in the cb.
>
> Thanks,
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4e09398009c1..6c020015d556 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct
> net
> *net, struct sk_buff *skb)
> {
> #if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
> if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
> - skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
> + skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
> return true;
> #endif
> return false;
I wonder if this should not be in a separate patch ?
Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
TCP_SKB_CB layout to reduce cache line misses") in linux 3.18
While VRF was added later.
If you agree, I will prepare a patch series, with different Fixes tag
so that David can decide which path needs to be backported into each
stable version.
Thanks.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 17:57 ` Eric Dumazet
0 siblings, 0 replies; 38+ messages in thread
From: Eric Dumazet @ 2017-11-30 17:57 UTC (permalink / raw)
To: linux-security-module
On Thu, 2017-11-30 at 10:30 -0700, David Ahern wrote:
> On 11/30/17 8:44 AM, David Ahern wrote:
> > On 11/30/17 3:50 AM, Eric Dumazet wrote:
> > > @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
> > > ?
> > > ? th = (const struct tcphdr *)skb->data;
> > > ? iph = ip_hdr(skb);
> > > - /* This is tricky : We move IPCB at its correct location
> > > into TCP_SKB_CB()
> > > - ?* barrier() makes sure compiler wont play
> > > fool^Waliasing games.
> > > - ?*/
> > > - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
> > > - sizeof(struct inet_skb_parm));
> > > - barrier();
> > > -
> > > - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> > > - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th-
> > > >syn + th->fin +
> > > - ????skb->len - th->doff * 4);
> > > - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> > > - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> > > - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> > > - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> > > - TCP_SKB_CB(skb)->sacked ?= 0;
> > > - TCP_SKB_CB(skb)->has_rxtstamp =
> > > - skb->tstamp || skb_hwtstamps(skb)-
> > > >hwtstamp;
> > > -
> > > ?lookup:
> > > ? sk = __inet_lookup_skb(&tcp_hashinfo, skb,
> > > __tcp_hdrlen(th), th->source,
> > > ? ???????th->dest, sdif, &refcounted);
> >
> > I believe moving the above is going to affect lookups with VRF. Let
> > me
> > take a look before this gets committed.
> >
>
> Eric:
>
> Can you add this to the patch? Fixes socket lookups with VRF which
> stashes a flag in the cb.
>
> Thanks,
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4e09398009c1..6c020015d556 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct
> net
> *net, struct sk_buff *skb)
> ?{
> ?#if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
> ????????if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
> -???????????skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
> +???????????skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
> ????????????????return true;
> ?#endif
> ????????return false;
I wonder if this should not be in a separate patch ?
Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
TCP_SKB_CB layout to reduce cache line misses") in linux 3.18
While VRF was added later.
If you agree, I will prepare a patch series, with different Fixes tag
so that David can decide which path needs to be backported into each
stable version.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 17:57 ` Eric Dumazet
@ 2017-11-30 18:03 ` David Ahern
-1 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2017-11-30 18:03 UTC (permalink / raw)
To: Eric Dumazet, Casey Schaufler, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On 11/30/17 10:57 AM, Eric Dumazet wrote:
> I wonder if this should not be in a separate patch ?
>
> Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
> TCP_SKB_CB layout to reduce cache line misses") in linux 3.18
>
> While VRF was added later.
>
> If you agree, I will prepare a patch series, with different Fixes tag
> so that David can decide which path needs to be backported into each
> stable version.
>
That's sound fine to me.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 18:03 ` David Ahern
0 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2017-11-30 18:03 UTC (permalink / raw)
To: linux-security-module
On 11/30/17 10:57 AM, Eric Dumazet wrote:
> I wonder if this should not be in a separate patch ?
>
> Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
> TCP_SKB_CB layout to reduce cache line misses") in linux 3.18
>
> While VRF was added later.
>
> If you agree, I will prepare a patch series, with different Fixes tag
> so that David can decide which path needs to be backported into each
> stable version.
>
That's sound fine to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 17:57 ` Eric Dumazet
@ 2017-11-30 18:16 ` Casey Schaufler
-1 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 18:16 UTC (permalink / raw)
To: Eric Dumazet, David Ahern, James Morris
Cc: Paul Moore, netdev, Stephen Smalley, selinux, LSM
On 11/30/2017 9:57 AM, Eric Dumazet wrote:
> On Thu, 2017-11-30 at 10:30 -0700, David Ahern wrote:
>> On 11/30/17 8:44 AM, David Ahern wrote:
>>> On 11/30/17 3:50 AM, Eric Dumazet wrote:
>>>> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>>>
>>>> th = (const struct tcphdr *)skb->data;
>>>> iph = ip_hdr(skb);
>>>> - /* This is tricky : We move IPCB at its correct location
>>>> into TCP_SKB_CB()
>>>> - * barrier() makes sure compiler wont play
>>>> fool^Waliasing games.
>>>> - */
>>>> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>>>> - sizeof(struct inet_skb_parm));
>>>> - barrier();
>>>> -
>>>> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>>>> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th-
>>>>> syn + th->fin +
>>>> - skb->len - th->doff * 4);
>>>> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>>>> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>>>> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>>>> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>>>> - TCP_SKB_CB(skb)->sacked = 0;
>>>> - TCP_SKB_CB(skb)->has_rxtstamp =
>>>> - skb->tstamp || skb_hwtstamps(skb)-
>>>>> hwtstamp;
>>>> -
>>>> lookup:
>>>> sk = __inet_lookup_skb(&tcp_hashinfo, skb,
>>>> __tcp_hdrlen(th), th->source,
>>>> th->dest, sdif, &refcounted);
>>> I believe moving the above is going to affect lookups with VRF. Let
>>> me
>>> take a look before this gets committed.
>>>
>> Eric:
>>
>> Can you add this to the patch? Fixes socket lookups with VRF which
>> stashes a flag in the cb.
I've done my testing and it works both ways for Smack.
>>
>> Thanks,
>>
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 4e09398009c1..6c020015d556 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct
>> net
>> *net, struct sk_buff *skb)
>> {
>> #if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
>> if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
>> - skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
>> + skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
>> return true;
>> #endif
>> return false;
>
> I wonder if this should not be in a separate patch ?
>
> Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
> TCP_SKB_CB layout to reduce cache line misses") in linux 3.18
>
> While VRF was added later.
>
> If you agree, I will prepare a patch series, with different Fixes tag
> so that David can decide which path needs to be backported into each
> stable version.
>
> Thanks.
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-11-30 18:16 ` Casey Schaufler
0 siblings, 0 replies; 38+ messages in thread
From: Casey Schaufler @ 2017-11-30 18:16 UTC (permalink / raw)
To: linux-security-module
On 11/30/2017 9:57 AM, Eric Dumazet wrote:
> On Thu, 2017-11-30 at 10:30 -0700, David Ahern wrote:
>> On 11/30/17 8:44 AM, David Ahern wrote:
>>> On 11/30/17 3:50 AM, Eric Dumazet wrote:
>>>> @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>>> ?
>>>> ? th = (const struct tcphdr *)skb->data;
>>>> ? iph = ip_hdr(skb);
>>>> - /* This is tricky : We move IPCB at its correct location
>>>> into TCP_SKB_CB()
>>>> - ?* barrier() makes sure compiler wont play
>>>> fool^Waliasing games.
>>>> - ?*/
>>>> - memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
>>>> - sizeof(struct inet_skb_parm));
>>>> - barrier();
>>>> -
>>>> - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
>>>> - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th-
>>>>> syn + th->fin +
>>>> - ????skb->len - th->doff * 4);
>>>> - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
>>>> - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
>>>> - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
>>>> - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
>>>> - TCP_SKB_CB(skb)->sacked ?= 0;
>>>> - TCP_SKB_CB(skb)->has_rxtstamp =
>>>> - skb->tstamp || skb_hwtstamps(skb)-
>>>>> hwtstamp;
>>>> -
>>>> ?lookup:
>>>> ? sk = __inet_lookup_skb(&tcp_hashinfo, skb,
>>>> __tcp_hdrlen(th), th->source,
>>>> ? ???????th->dest, sdif, &refcounted);
>>> I believe moving the above is going to affect lookups with VRF. Let
>>> me
>>> take a look before this gets committed.
>>>
>> Eric:
>>
>> Can you add this to the patch? Fixes socket lookups with VRF which
>> stashes a flag in the cb.
I've done my testing and it works both ways for Smack.
>>
>> Thanks,
>>
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 4e09398009c1..6c020015d556 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct
>> net
>> *net, struct sk_buff *skb)
>> ?{
>> ?#if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
>> ????????if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
>> -???????????skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
>> +???????????skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
>> ????????????????return true;
>> ?#endif
>> ????????return false;
>
> I wonder if this should not be in a separate patch ?
>
> Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
> TCP_SKB_CB layout to reduce cache line misses") in linux 3.18
>
> While VRF was added later.
>
> If you agree, I will prepare a patch series, with different Fixes tag
> so that David can decide which path needs to be backported into each
> stable version.
>
> Thanks.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [BUG] kernel stack corruption during/after Netlabel error
2017-11-30 10:50 ` Eric Dumazet
@ 2017-12-01 1:55 ` James Morris
-1 siblings, 0 replies; 38+ messages in thread
From: James Morris @ 2017-12-01 1:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: Casey Schaufler, Paul Moore, netdev, Stephen Smalley, selinux, LSM
On Thu, 30 Nov 2017, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
> > On 11/29/2017 4:31 PM, James Morris wrote:
> > > On Wed, 29 Nov 2017, Casey Schaufler wrote:
> > >
> > > > I see that there is a proposed fix later in the thread, but I
> > > > don't see
> > > > the patch. Could you send it to me, so I can try it on my
> > > > problem?
> > >
> > > Forwarded off-list.
> >
> > The patch does fix the problem I was seeing in Smack.
>
> Can you guys test the following more complete patch ?
>
> It should cover IPv4 and IPv6, and also the corner cases.
Tested-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [BUG] kernel stack corruption during/after Netlabel error
@ 2017-12-01 1:55 ` James Morris
0 siblings, 0 replies; 38+ messages in thread
From: James Morris @ 2017-12-01 1:55 UTC (permalink / raw)
To: linux-security-module
On Thu, 30 Nov 2017, Eric Dumazet wrote:
> On Wed, 2017-11-29 at 19:16 -0800, Casey Schaufler wrote:
> > On 11/29/2017 4:31 PM, James Morris wrote:
> > > On Wed, 29 Nov 2017, Casey Schaufler wrote:
> > >
> > > > I see that there is a proposed fix later in the thread, but I
> > > > don't see
> > > > the patch. Could you send it to me, so I can try it on my
> > > > problem?
> > >
> > > Forwarded off-list.
> >
> > The patch does fix the problem I was seeing in Smack.
>
> Can you guys test the following more complete patch ?
>
> It should cover IPv4 and IPv6, and also the corner cases.
Tested-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2017-12-01 1:55 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 10:26 [BUG] kernel stack corruption during/after Netlabel error James Morris
2017-11-29 12:29 ` Eric Dumazet
2017-11-29 17:31 ` Stephen Smalley
2017-11-29 17:34 ` Eric Dumazet
2017-11-29 19:29 ` Paul Moore
[not found] ` <CANn89iJc=tZkN41WoCm5Zy9nPfs1tfZf9nuSXYS9EB_aem+y4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-29 19:59 ` Stephen Smalley
2017-11-29 19:59 ` Stephen Smalley
2017-11-29 20:23 ` Eric Dumazet
2017-11-29 22:49 ` Eric Dumazet
2017-11-29 23:41 ` James Morris
2017-11-30 0:22 ` Casey Schaufler
2017-11-30 0:22 ` Casey Schaufler
2017-11-30 0:31 ` James Morris
2017-11-30 0:31 ` James Morris
2017-11-30 3:16 ` Casey Schaufler
2017-11-30 3:16 ` Casey Schaufler
2017-11-30 10:50 ` Eric Dumazet
2017-11-30 10:50 ` Eric Dumazet
2017-11-30 12:47 ` Paul Moore
2017-11-30 12:47 ` Paul Moore
2017-11-30 16:57 ` Paul Moore
2017-11-30 16:57 ` Paul Moore
2017-11-30 14:33 ` Casey Schaufler
2017-11-30 14:33 ` Casey Schaufler
2017-11-30 15:11 ` Casey Schaufler
2017-11-30 15:11 ` Casey Schaufler
2017-11-30 15:44 ` David Ahern
2017-11-30 15:44 ` David Ahern
2017-11-30 17:30 ` David Ahern
2017-11-30 17:30 ` David Ahern
2017-11-30 17:57 ` Eric Dumazet
2017-11-30 17:57 ` Eric Dumazet
2017-11-30 18:03 ` David Ahern
2017-11-30 18:03 ` David Ahern
2017-11-30 18:16 ` Casey Schaufler
2017-11-30 18:16 ` Casey Schaufler
2017-12-01 1:55 ` James Morris
2017-12-01 1:55 ` James Morris
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.