| From 5fa782c2f5ef6c2e4f04d3e228412c9b4a4c8809 Mon Sep 17 00:00:00 2001 |
| From: Neil Horman <nhorman@tuxdriver.com> |
| Date: Wed, 28 Apr 2010 10:30:59 +0000 |
| Subject: sctp: Fix skb_over_panic resulting from multiple invalid parameter errors (CVE-2010-1173) (v4) |
| |
| From: Neil Horman <nhorman@tuxdriver.com> |
| |
| commit 5fa782c2f5ef6c2e4f04d3e228412c9b4a4c8809 upstream. |
| |
| Ok, version 4 |
| |
| Change Notes: |
| 1) Minor cleanups, from Vlads notes |
| |
| Summary: |
| |
| Hey- |
| Recently, it was reported to me that the kernel could oops in the |
| following way: |
| |
| <5> kernel BUG at net/core/skbuff.c:91! |
| <5> invalid operand: 0000 [#1] |
| <5> Modules linked in: sctp netconsole nls_utf8 autofs4 sunrpc iptable_filter |
| ip_tables cpufreq_powersave parport_pc lp parport vmblock(U) vsock(U) vmci(U) |
| vmxnet(U) vmmemctl(U) vmhgfs(U) acpiphp dm_mirror dm_mod button battery ac md5 |
| ipv6 uhci_hcd ehci_hcd snd_ens1371 snd_rawmidi snd_seq_device snd_pcm_oss |
| snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_ac97_codec snd soundcore |
| pcnet32 mii floppy ext3 jbd ata_piix libata mptscsih mptsas mptspi mptscsi |
| mptbase sd_mod scsi_mod |
| <5> CPU: 0 |
| <5> EIP: 0060:[<c02bff27>] Not tainted VLI |
| <5> EFLAGS: 00010216 (2.6.9-89.0.25.EL) |
| <5> EIP is at skb_over_panic+0x1f/0x2d |
| <5> eax: 0000002c ebx: c033f461 ecx: c0357d96 edx: c040fd44 |
| <5> esi: c033f461 edi: df653280 ebp: 00000000 esp: c040fd40 |
| <5> ds: 007b es: 007b ss: 0068 |
| <5> Process swapper (pid: 0, threadinfo=c040f000 task=c0370be0) |
| <5> Stack: c0357d96 e0c29478 00000084 00000004 c033f461 df653280 d7883180 |
| e0c2947d |
| <5> 00000000 00000080 df653490 00000004 de4f1ac0 de4f1ac0 00000004 |
| df653490 |
| <5> 00000001 e0c2877a 08000800 de4f1ac0 df653490 00000000 e0c29d2e |
| 00000004 |
| <5> Call Trace: |
| <5> [<e0c29478>] sctp_addto_chunk+0xb0/0x128 [sctp] |
| <5> [<e0c2947d>] sctp_addto_chunk+0xb5/0x128 [sctp] |
| <5> [<e0c2877a>] sctp_init_cause+0x3f/0x47 [sctp] |
| <5> [<e0c29d2e>] sctp_process_unk_param+0xac/0xb8 [sctp] |
| <5> [<e0c29e90>] sctp_verify_init+0xcc/0x134 [sctp] |
| <5> [<e0c20322>] sctp_sf_do_5_1B_init+0x83/0x28e [sctp] |
| <5> [<e0c25333>] sctp_do_sm+0x41/0x77 [sctp] |
| <5> [<c01555a4>] cache_grow+0x140/0x233 |
| <5> [<e0c26ba1>] sctp_endpoint_bh_rcv+0xc5/0x108 [sctp] |
| <5> [<e0c2b863>] sctp_inq_push+0xe/0x10 [sctp] |
| <5> [<e0c34600>] sctp_rcv+0x454/0x509 [sctp] |
| <5> [<e084e017>] ipt_hook+0x17/0x1c [iptable_filter] |
| <5> [<c02d005e>] nf_iterate+0x40/0x81 |
| <5> [<c02e0bb9>] ip_local_deliver_finish+0x0/0x151 |
| <5> [<c02e0c7f>] ip_local_deliver_finish+0xc6/0x151 |
| <5> [<c02d0362>] nf_hook_slow+0x83/0xb5 |
| <5> [<c02e0bb2>] ip_local_deliver+0x1a2/0x1a9 |
| <5> [<c02e0bb9>] ip_local_deliver_finish+0x0/0x151 |
| <5> [<c02e103e>] ip_rcv+0x334/0x3b4 |
| <5> [<c02c66fd>] netif_receive_skb+0x320/0x35b |
| <5> [<e0a0928b>] init_stall_timer+0x67/0x6a [uhci_hcd] |
| <5> [<c02c67a4>] process_backlog+0x6c/0xd9 |
| <5> [<c02c690f>] net_rx_action+0xfe/0x1f8 |
| <5> [<c012a7b1>] __do_softirq+0x35/0x79 |
| <5> [<c0107efb>] handle_IRQ_event+0x0/0x4f |
| <5> [<c01094de>] do_softirq+0x46/0x4d |
| |
| Its an skb_over_panic BUG halt that results from processing an init chunk in |
| which too many of its variable length parameters are in some way malformed. |
| |
| The problem is in sctp_process_unk_param: |
| if (NULL == *errp) |
| *errp = sctp_make_op_error_space(asoc, chunk, |
| ntohs(chunk->chunk_hdr->length)); |
| |
| if (*errp) { |
| sctp_init_cause(*errp, SCTP_ERROR_UNKNOWN_PARAM, |
| WORD_ROUND(ntohs(param.p->length))); |
| sctp_addto_chunk(*errp, |
| WORD_ROUND(ntohs(param.p->length)), |
| param.v); |
| |
| When we allocate an error chunk, we assume that the worst case scenario requires |
| that we have chunk_hdr->length data allocated, which would be correct nominally, |
| given that we call sctp_addto_chunk for the violating parameter. Unfortunately, |
| we also, in sctp_init_cause insert a sctp_errhdr_t structure into the error |
| chunk, so the worst case situation in which all parameters are in violation |
| requires chunk_hdr->length+(sizeof(sctp_errhdr_t)*param_count) bytes of data. |
| |
| The result of this error is that a deliberately malformed packet sent to a |
| listening host can cause a remote DOS, described in CVE-2010-1173: |
| http://cve.mitre.org/cgi-bin/cvename.cgi?name=2010-1173 |
| |
| I've tested the below fix and confirmed that it fixes the issue. We move to a |
| strategy whereby we allocate a fixed size error chunk and ignore errors we don't |
| have space to report. Tested by me successfully |
| |
| Signed-off-by: Neil Horman <nhorman@tuxdriver.com> |
| Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| include/net/sctp/structs.h | 1 |
| net/sctp/sm_make_chunk.c | 62 +++++++++++++++++++++++++++++++++++++++++---- |
| 2 files changed, 58 insertions(+), 5 deletions(-) |
| |
| --- a/include/net/sctp/structs.h |
| +++ b/include/net/sctp/structs.h |
| @@ -772,6 +772,7 @@ int sctp_user_addto_chunk(struct sctp_ch |
| struct iovec *data); |
| void sctp_chunk_free(struct sctp_chunk *); |
| void *sctp_addto_chunk(struct sctp_chunk *, int len, const void *data); |
| +void *sctp_addto_chunk_fixed(struct sctp_chunk *, int len, const void *data); |
| struct sctp_chunk *sctp_chunkify(struct sk_buff *, |
| const struct sctp_association *, |
| struct sock *); |
| --- a/net/sctp/sm_make_chunk.c |
| +++ b/net/sctp/sm_make_chunk.c |
| @@ -107,7 +107,7 @@ static const struct sctp_paramhdr prsctp |
| cpu_to_be16(sizeof(struct sctp_paramhdr)), |
| }; |
| |
| -/* A helper to initialize to initialize an op error inside a |
| +/* A helper to initialize an op error inside a |
| * provided chunk, as most cause codes will be embedded inside an |
| * abort chunk. |
| */ |
| @@ -124,6 +124,29 @@ void sctp_init_cause(struct sctp_chunk |
| chunk->subh.err_hdr = sctp_addto_chunk(chunk, sizeof(sctp_errhdr_t), &err); |
| } |
| |
| +/* A helper to initialize an op error inside a |
| + * provided chunk, as most cause codes will be embedded inside an |
| + * abort chunk. Differs from sctp_init_cause in that it won't oops |
| + * if there isn't enough space in the op error chunk |
| + */ |
| +int sctp_init_cause_fixed(struct sctp_chunk *chunk, __be16 cause_code, |
| + size_t paylen) |
| +{ |
| + sctp_errhdr_t err; |
| + __u16 len; |
| + |
| + /* Cause code constants are now defined in network order. */ |
| + err.cause = cause_code; |
| + len = sizeof(sctp_errhdr_t) + paylen; |
| + err.length = htons(len); |
| + |
| + if (skb_tailroom(chunk->skb) > len) |
| + return -ENOSPC; |
| + chunk->subh.err_hdr = sctp_addto_chunk_fixed(chunk, |
| + sizeof(sctp_errhdr_t), |
| + &err); |
| + return 0; |
| +} |
| /* 3.3.2 Initiation (INIT) (1) |
| * |
| * This chunk is used to initiate a SCTP association between two |
| @@ -1125,6 +1148,24 @@ nodata: |
| return retval; |
| } |
| |
| +/* Create an Operation Error chunk of a fixed size, |
| + * specifically, max(asoc->pathmtu, SCTP_DEFAULT_MAXSEGMENT) |
| + * This is a helper function to allocate an error chunk for |
| + * for those invalid parameter codes in which we may not want |
| + * to report all the errors, if the incomming chunk is large |
| + */ |
| +static inline struct sctp_chunk *sctp_make_op_error_fixed( |
| + const struct sctp_association *asoc, |
| + const struct sctp_chunk *chunk) |
| +{ |
| + size_t size = asoc ? asoc->pathmtu : 0; |
| + |
| + if (!size) |
| + size = SCTP_DEFAULT_MAXSEGMENT; |
| + |
| + return sctp_make_op_error_space(asoc, chunk, size); |
| +} |
| + |
| /* Create an Operation Error chunk. */ |
| struct sctp_chunk *sctp_make_op_error(const struct sctp_association *asoc, |
| const struct sctp_chunk *chunk, |
| @@ -1365,6 +1406,18 @@ void *sctp_addto_chunk(struct sctp_chunk |
| return target; |
| } |
| |
| +/* Append bytes to the end of a chunk. Returns NULL if there isn't sufficient |
| + * space in the chunk |
| + */ |
| +void *sctp_addto_chunk_fixed(struct sctp_chunk *chunk, |
| + int len, const void *data) |
| +{ |
| + if (skb_tailroom(chunk->skb) > len) |
| + return sctp_addto_chunk(chunk, len, data); |
| + else |
| + return NULL; |
| +} |
| + |
| /* Append bytes from user space to the end of a chunk. Will panic if |
| * chunk is not big enough. |
| * Returns a kernel err value. |
| @@ -1968,13 +2021,12 @@ static sctp_ierror_t sctp_process_unk_pa |
| * returning multiple unknown parameters. |
| */ |
| if (NULL == *errp) |
| - *errp = sctp_make_op_error_space(asoc, chunk, |
| - ntohs(chunk->chunk_hdr->length)); |
| + *errp = sctp_make_op_error_fixed(asoc, chunk); |
| |
| if (*errp) { |
| - sctp_init_cause(*errp, SCTP_ERROR_UNKNOWN_PARAM, |
| + sctp_init_cause_fixed(*errp, SCTP_ERROR_UNKNOWN_PARAM, |
| WORD_ROUND(ntohs(param.p->length))); |
| - sctp_addto_chunk(*errp, |
| + sctp_addto_chunk_fixed(*errp, |
| WORD_ROUND(ntohs(param.p->length)), |
| param.v); |
| } else { |