Dapper CVE-2010-3873, memory corruption in X.25 facilities parsing
Tim Gardner
tim.gardner at canonical.com
Mon Jan 31 17:01:16 UTC 2011
On 01/31/2011 03:25 AM, Andy Whitcroft wrote:
> On Fri, Jan 28, 2011 at 01:35:26PM -0700, Tim Gardner wrote:
>> The following changes since commit 9f646039dcba8632e8d68f73652ce6949d6b20ba:
>> David S. Miller (1):
>> net: Limit socket I/O iovec total length to INT_MAX., CVE-2010-3859
>>
>> are available in the git repository at:
>>
>> git://kernel.ubuntu.com/rtg/ubuntu-dapper.git CVE-2010-3873
>>
>> Tim Gardner (1):
>> memory corruption in X.25 facilities parsing, CVE-2010-3873
>>
>> net/x25/x25_in.c | 10 +++++++---
>> 1 files changed, 7 insertions(+), 3 deletions(-)
>>
>> From 92700cf43dcb6ffdd998550a2ecadb7a2343e222 Mon Sep 17 00:00:00 2001
>> From: Tim Gardner<tim.gardner at canonical.com>
>> Date: Fri, 28 Jan 2011 11:17:01 -0700
>> Subject: [PATCH] memory corruption in X.25 facilities parsing, CVE-2010-3873
>>
>> BugLink: http://bugs.launchpad.net/bugs/709372
>>
>> CVE-2010-3873
>>
>> Partial backport from a6331d6f9a4298173b413cf99a40cc86a9d92c37
>> by Tim Gardner<tim.gardner at canonical.com>
>>
>> Signed-of-by: Andrew Hendry<andrew.hendry at gmail.com>
>>
>> Signed-off-by: David S. Miller<davem at davemloft.net>
>> Signed-off-by: Tim Gardner<tim.gardner at canonical.com>
>> ---
>> net/x25/x25_in.c | 10 +++++++---
>> 1 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
>> index 2614687..659252b 100644
>> --- a/net/x25/x25_in.c
>> +++ b/net/x25/x25_in.c
>> @@ -90,6 +90,7 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
>> switch (frametype) {
>> case X25_CALL_ACCEPTED: {
>> struct x25_sock *x25 = x25_sk(sk);
>> + int len;
>>
>> x25_stop_timer(sk);
>> x25->condition = 0x00;
>> @@ -104,9 +105,12 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
>> */
>> skb_pull(skb, X25_STD_MIN_LEN);
>> skb_pull(skb, x25_addr_ntoa(skb->data,&source_addr,&dest_addr));
>> - skb_pull(skb,
>> - x25_parse_facilities(skb,&x25->facilities,
>> - &x25->vc_facil_mask));
>> + len = x25_parse_facilities(skb,&x25->facilities,
>> + &x25->vc_facil_mask);
>> + if (len<= 0)
>> + return -1;
>> + skb_pull(skb, len);
>> +
>> /*
>> * Copy any Call User Data.
>> */
>
> As we do not support CLASS_D at all in Dapper it is not clear that we
> can ever return a negative or zero length from x25_parse_facilities().
> This is however safe and matches upstream.
>
> I do have one concern, and this applies to the change as applied to
> upstream and not a specific concern with any of the backports. I think
> that returning -1 here will trigger the calling backlog handler to think
> that we have queued the packet for further processing (which we have not)
> and thus we will leak the skb in the code below:
>
> int x25_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> {
> int queued = x25_process_rx_frame(sk, skb);
>
> if (!queued)
> kfree_skb(skb);
>
> return 0;
> }
>
> I will separatly ask about this upstream, as this may mean we are
> replacing a remote corrupter with a remote DOS.
>
> -apw
>
Your observation seems obviously correct to me. Without waiting for
upstream, how about the attached ?
rtg
--
Tim Gardner tim.gardner at canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-memory-corruption-in-X.25-facilities-parsing-CVE-201.patch
Type: text/x-patch
Size: 1563 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20110131/706815cc/attachment.bin>
More information about the kernel-team
mailing list