* Crash in CIPSO_V4_TAG_LOCAL handling
@ 2012-07-13 17:08 Alan Cox
0 siblings, 0 replies; 5+ messages in thread
From: Alan Cox @ 2012-07-13 17:08 UTC (permalink / raw)
To: netdev
DaveM asked that this goes direct to the list so ...
Looking at some static code analyser output and the results seem valid on
this one:
ip_options_compile called with skb = NULL and an IPOPT_CIPSO option calls
into cipso_v4_validate, which if the option is CIPSO_V4_TAG_LOCAL then
dereferences skb->dev and explodes.
All the other cases appear to correctly avoid or check it versus NULL
Alan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Crash in CIPSO_V4_TAG_LOCAL handling
@ 2012-07-14 17:22 Lin Ming
2012-07-14 20:08 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Lin Ming @ 2012-07-14 17:22 UTC (permalink / raw)
To: Alan Cox, Paul Moore; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]
On Sat, Jul 14, 2012 at 1:08 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> DaveM asked that this goes direct to the list so ...
>
> Looking at some static code analyser output and the results seem valid on
> this one:
>
> ip_options_compile called with skb = NULL and an IPOPT_CIPSO option calls
> into cipso_v4_validate, which if the option is CIPSO_V4_TAG_LOCAL then
> dereferences skb->dev and explodes.
Hi Alan,
I can trigger this oops with the attached simple program(cipso_test.c).
# netlabelctl cipsov4 add local doi:3
# ./cipso_test
It's caused by below code added in commit 15c45f7b.
case CIPSO_V4_TAG_LOCAL:
/* This is a non-standard tag that we only allow for
* local connections, so if the incoming interface is
* not the loopback device drop the packet. */
if (!(skb->dev->flags & IFF_LOOPBACK)) {
err_offset = opt_iter;
goto validate_return_locked;
}
But I don't know how to check if it is loopback device
just after socket() syscall.
sockfd = socket(AF_INET, SOCK_DGRAM, 0);
setsockopt(sockfd, SOL_IP, IP_OPTIONS, &cipso, sizeof(struct cipso));
Obviously, at this time net_device is not assigned yet.
Any idea?
[ 346.396250] BUG: unable to handle kernel NULL pointer dereference at 00000014
[ 346.400024] IP: [<c171041b>] cipso_v4_validate+0x323/0x363
[ 346.400024] *pdpt = 000000001efe4001 *pde = 0000000000000000
[ 346.400024] Oops: 0000 [#1] SMP
[ 346.400024] Modules linked in:
[ 346.400024]
[ 346.400024] Pid: 2471, comm: cipso_test Not tainted 3.5.0-rc5-01438-gd322ef7 #28 Bochs Bochs
[ 346.400024] EIP: 0060:[<c171041b>] EFLAGS: 00010246 CPU: 0
[ 346.400024] EIP is at cipso_v4_validate+0x323/0x363
[ 346.400024] EAX: 00000006 EBX: de77d4de ECX: 00000006 EDX: de77d380
[ 346.400024] ESI: 00000000 EDI: 00000006 EBP: df61dcb8 ESP: df61dc5c
[ 346.400024] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 346.400024] CR0: 8005003b CR2: 00000014 CR3: 1efac000 CR4: 000006b0
[ 346.400024] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 346.400024] DR6: ffff0ff0 DR7: 00000400
[ 346.400024] Process cipso_test (pid: 2471, ti=df61c000 task=df7f0530 task.ti=df61c000)
[ 346.400024] Stack:
[ 346.400024] 000000d0 00000040 df404600 de7debe0 0001cd2c df61dc8c 0000000c 0000001c
[ 346.400024] 00000000 00000001 c1b2606c df61dc98 c12dfa20 df61dcf4 0c7f0530 de77d4d8
[ 346.400024] c106c380 06f73800 00000006 de77d398 de77d4c8 00000000 0000000c df61dd04
[ 346.400024] Call Trace:
[ 346.400024] [<c12dfa20>] ? security_capable+0x1c/0x20
[ 346.400024] [<c106c380>] ? sys_sysctl+0x110/0x140
[ 346.400024] [<c16d2370>] ip_options_compile+0x3c4/0x423
[ 346.400024] [<c10f0999>] ? unlock_page+0x46/0x49
[ 346.400024] [<c110b5ef>] ? __do_fault+0x460/0x474
[ 346.400024] [<c16d1d57>] ? ip_options_get_alloc+0x1b/0x1d
[ 346.400024] [<c16d2401>] ip_options_get_finish+0x32/0x53
[ 346.400024] [<c16d24cf>] ip_options_get_from_user+0x5d/0x65
[ 346.400024] [<c16d5453>] do_ip_setsockopt+0xe3/0xa97
[ 346.400024] [<c105de56>] ? kmap_atomic_prot+0x110/0x112
[ 346.400024] [<c10f2f12>] ? zone_watermark_ok+0x30/0x37
[ 346.400024] [<c10f58e9>] ? get_page_from_freelist+0x38b/0x42c
[ 346.400024] [<c10ef736>] ? find_get_page+0x22/0x6d
[ 346.400024] [<c10f0d2f>] ? filemap_fault+0x7a/0x31a
[ 346.400024] [<c10f0999>] ? unlock_page+0x46/0x49
[ 346.400024] [<c110b5c7>] ? __do_fault+0x438/0x474
[ 346.400024] [<c1664565>] ? sk_prot_alloc+0x25/0xb8
[ 346.400024] [<c16645f8>] ? sk_prot_alloc+0xb8/0xb8
[ 346.400024] [<c16645c7>] ? sk_prot_alloc+0x87/0xb8
[ 346.400024] [<c12dfca1>] ? security_file_alloc+0x14/0x16
[ 346.400024] [<c11333c0>] ? get_empty_filp+0xfb/0x1cd
[ 346.400024] [<c16d5e31>] ip_setsockopt+0x2a/0x8c
[ 346.400024] [<c16ef873>] udp_setsockopt+0x42/0x49
[ 346.400024] [<c1662b91>] sock_common_setsockopt+0x23/0x29
[ 346.400024] [<c16613fa>] sys_setsockopt+0x6e/0x8a
[ 346.400024] [<c16627c4>] sys_socketcall+0x1fa/0x2bc
[ 346.400024] [<c1311ea0>] ? trace_hardirqs_on_thunk+0xc/0x10
[ 346.400024] [<c180f878>] sysenter_do_call+0x12/0x2d
[ 346.400024] Code: 3b 4d ec 77 09 66 8b 74 0b 02 66 c1 c6 08 0f b7 ff 3b 7d e4 77 0d 0f b7 f6 89 75 e4 3b 4d ec 72 d1 eb 1d 83 c0 04 eb 2e 8b 75 c4 <8b> 4e 14 f6 81 e4 00 00 00 08 74 1f 80 7d eb 06 74 03 40 eb 16
[ 346.400024] EIP: [<c171041b>] cipso_v4_validate+0x323/0x363 SS:ESP 0068:df61dc5c
>
> All the other cases appear to correctly avoid or check it versus NULL
>
> Alan
[-- Attachment #2: cipso_test.c --]
[-- Type: text/x-csrc, Size: 660 bytes --]
#include <sys/types.h>
#include <sys/socket.h>
#include <linux/ip.h>
#include <linux/in.h>
#include <string.h>
struct local_tag {
char type;
char length;
char info[4];
};
struct cipso {
char type;
char length;
char doi[4];
struct local_tag local;
};
int main(int argc, char **argv)
{
int sockfd;
struct cipso cipso = {
.type = IPOPT_CIPSO,
.length = sizeof(struct cipso),
.local = {
.type = 128,
.length = sizeof(struct local_tag),
},
};
memset(cipso.doi, 0, 4);
cipso.doi[3] = 3;
sockfd = socket(AF_INET, SOCK_DGRAM, 0);
#define SOL_IP 0
setsockopt(sockfd, SOL_IP, IP_OPTIONS, &cipso, sizeof(struct cipso));
return 0;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Crash in CIPSO_V4_TAG_LOCAL handling
2012-07-14 17:22 Crash in CIPSO_V4_TAG_LOCAL handling Lin Ming
@ 2012-07-14 20:08 ` David Miller
2012-07-17 16:25 ` Paul Moore
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2012-07-14 20:08 UTC (permalink / raw)
To: mlin; +Cc: alan, paul.moore, netdev
From: Lin Ming <mlin@ss.pku.edu.cn>
Date: Sun, 15 Jul 2012 01:22:30 +0800
> It's caused by below code added in commit 15c45f7b.
>
> case CIPSO_V4_TAG_LOCAL:
> /* This is a non-standard tag that we only allow for
> * local connections, so if the incoming interface is
> * not the loopback device drop the packet. */
> if (!(skb->dev->flags & IFF_LOOPBACK)) {
> err_offset = opt_iter;
> goto validate_return_locked;
> }
Paul please fix this, as shown 'skb' can easily be NULL in this
code path.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Crash in CIPSO_V4_TAG_LOCAL handling
2012-07-14 20:08 ` David Miller
@ 2012-07-17 16:25 ` Paul Moore
2012-07-17 17:28 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Paul Moore @ 2012-07-17 16:25 UTC (permalink / raw)
To: David Miller; +Cc: mlin, alan, netdev
On Sat, Jul 14, 2012 at 4:08 PM, David Miller <davem@davemloft.net> wrote:
> From: Lin Ming <mlin@ss.pku.edu.cn>
> Date: Sun, 15 Jul 2012 01:22:30 +0800
>
>> It's caused by below code added in commit 15c45f7b.
>>
>> case CIPSO_V4_TAG_LOCAL:
>> /* This is a non-standard tag that we only allow for
>> * local connections, so if the incoming interface is
>> * not the loopback device drop the packet. */
>> if (!(skb->dev->flags & IFF_LOOPBACK)) {
>> err_offset = opt_iter;
>> goto validate_return_locked;
>> }
>
> Paul please fix this, as shown 'skb' can easily be NULL in this
> code path.
Just saw this ... I'll start looking into this today.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Crash in CIPSO_V4_TAG_LOCAL handling
2012-07-17 16:25 ` Paul Moore
@ 2012-07-17 17:28 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2012-07-17 17:28 UTC (permalink / raw)
To: paul; +Cc: mlin, alan, netdev
From: Paul Moore <paul@paul-moore.com>
Date: Tue, 17 Jul 2012 12:25:28 -0400
> On Sat, Jul 14, 2012 at 4:08 PM, David Miller <davem@davemloft.net> wrote:
>> From: Lin Ming <mlin@ss.pku.edu.cn>
>> Date: Sun, 15 Jul 2012 01:22:30 +0800
>>
>>> It's caused by below code added in commit 15c45f7b.
>>>
>>> case CIPSO_V4_TAG_LOCAL:
>>> /* This is a non-standard tag that we only allow for
>>> * local connections, so if the incoming interface is
>>> * not the loopback device drop the packet. */
>>> if (!(skb->dev->flags & IFF_LOOPBACK)) {
>>> err_offset = opt_iter;
>>> goto validate_return_locked;
>>> }
>>
>> Paul please fix this, as shown 'skb' can easily be NULL in this
>> code path.
>
> Just saw this ... I'll start looking into this today.
Thanks, sorry I messed up your email, I should have checked MAINTAINERS :)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-17 17:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-14 17:22 Crash in CIPSO_V4_TAG_LOCAL handling Lin Ming
2012-07-14 20:08 ` David Miller
2012-07-17 16:25 ` Paul Moore
2012-07-17 17:28 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2012-07-13 17:08 Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).