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
| ||
|
Date: Wed, 22 Oct 2008 19:24:24 -0500 From: Alex Villacís Lasso <avillaci@...bo.fiec.espol.edu.ec> To: irda-users@...ts.sourceforge.net, netdev@...r.kernel.org, David Miller <davem@...emloft.net> Subject: Re: Regression: Recent networking (qdisc?) patches break irda_get_next_speed() David Miller escribió: > From: Alex Villacís Lasso <avillaci@...bo.fiec.espol.edu.ec> > Date: Tue, 21 Oct 2008 18:37:56 -0500 > > >> 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'. >> > > The SKB control block is not aliased. > > Once the packet is given to dev_queue_xmit() the packet scheduler > "owns" the control block of the SKB. > > What IRDA is doing is illegal, and breaks in other ways without the > commit in question. > > IRDA cannot depend upon the SKB control block not changing across > the dev_queue_xmit() call. > > Let me see if I understood. So the particular illegal thing the IRDA stack is doing is the access of the control block in the middle of the driver transmit routine (via irda_get_next_speed() and friends). This information should be stored somewhere else. Exactly *where* to store it is the main problem to solve. What is the proper way (if any) to store per-packet parameters (other than the payload itself) which are specific to a particular layer (IrDA in this case) and which are needed by drivers in order to work correctly? The control block gets overwritten by the time the driver proc (hard_start_xmit) is called, so this approach is now ruled out. I was thinking about storing a copy of the parameters (struct irda_skb_cb) as a header within the payload itself (skb->data[]), but I am not sure about whether this approach is a good design decision. I am open to suggestions on where to place the parameters. -- 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
Powered by blists - more mailing lists