netdev - Re: Regression: Recent networking (qdisc?) patches break irda_get_next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Oct 2008 18:37:56 -0500
From:	Alex Villacís Lasso 
	<avillaci@...bo.fiec.espol.edu.ec>
To:	netdev@...r.kernel.org, Jarek Poplawski <jarkao2@...il.com>,
	Jussi Kivilinna <jussi.kivilinna@...et.fi>
Subject: Re: Regression: Recent networking (qdisc?) patches break irda_get_next_speed()

Jarek Poplawski escribió:
> Alex Villacís Lasso wrote, On 10/21/2008 08:20 PM:
>
>   
>> A regression has been introduced in 2.6.27 in the networking code, which 
>> breaks the irda_get_next_speed() function used by some IrDA drivers, 
>> including ks959-sir, written by me. I have filed a bug at:
>>
>> http://bugzilla.kernel.org/show_bug.cgi?id=11795
>>
>> to keep track of this. I am still performing bisection to locate the 
>> exact commit that broke the code, but the bug involves an overwriting of 
>> the beginning of a structure with extraneous data that makes the 
>> LAP_MAGIC check fail. I have tried searching the netdev archives, but 
>> there is no mention at all of regressions caused by this code. Milan 
>> Plzik at irda-users mentions the same problem affecting pxaficp_ir, and 
>> mentions recent qdisc patches as possible culprits.  Vasily Khoruzhick 
>> reported this bug at irda-users, and a quick hack (not a proper fix) 
>> proposed by him involves padding "struct irda_skb_cb" with 4 bytes at 
>> the beginning of the structure to move everything else past the buggy 
>> scribbling.
>>     
>
>
> Looks like the patch below could fit to your description.
>
> Jarek P.
>
> commit 175f9c1bba9b825d22b142d183c9e175488b260c
> Author: Jussi Kivilinna <jussi.kivilinna@...et.fi>
> Date:   Sun Jul 20 00:08:47 2008 -0700
>
>     net_sched: Add size table for qdiscs
>
>     Add size table functions for qdiscs and calculate packet size in
>     qdisc_enqueue().
>
>     Based on patch by Patrick McHardy
>      http://marc.info/?l=linux-netdev&m=115201979221729&w=2
>
>     Signed-off-by: Jussi Kivilinna <jussi.kivilinna@...et.fi>
>     Signed-off-by: David S. Miller <davem@...emloft.net>
>
>
>   
So then, the bug is that the cb field in the struct sk_buff is being 
interpreted as both a struct qdisc_skb_cb and an struct irda_skb_cb, for 
the same instance of struct sk_buff. I have just started to review the 
suggested patch, but it seems that 'struct qdisc_skb_cb' was meant to be 
aliased against the data for other layers (as suggested by the presence 
of a 'char data[]' field). If so, how come only IrDA is affected? How 
come UDP, TCP, etc. not affected by this? On the other hand, if 
qdisc_skb_cb was not meant to be aliased, then the IrDA case was left 
out while converting the rest of the layers so that they will skip over 
the member 'pkt_len' of the 'struct qdisc_skb_cb'.

Or maybe all the other layers *are* being overwritten, and IrDA is the 
only one that has a 'magic' member to check against corruption... As far 
as I can see, no other layer has a member with an equivalent function.

-- 
perl -e '$x=2.4;print sprintf("%.0f + %.0f = %.0f\n",$x,$x,$x+$x);'

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html