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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 29 Jan 2014 07:02:25 -0800 From: Eric Dumazet <eric.dumazet@...il.com> To: Oliver Hartkopp <socketcan@...tkopp.net> Cc: David Miller <davem@...emloft.net>, Linux Netdev List <netdev@...r.kernel.org>, Andre Naujoks <nautsch2@...il.com> Subject: Re: [PATCH stable 3.11+] can: use private sk reference to detect originating socket On Wed, 2014-01-29 at 14:44 +0100, Oliver Hartkopp wrote: > Commit 376c7311bdb6 (net: add a temporary sanity check in skb_orphan()) > leads to a BUG in can_put_echo_skb() when skb_orphan() is executed. > > The use of skb->sk to detect the originating socket can lead to side effects > in net/core and net/sched. Therefore the reference needed in net/can/raw.c is > now stored in a CAN private skbuff space in struct can_skb_priv. > > The reference which comes with the CAN frame is only needed in raw_rcv() and > checked against his own sock pointer and does only special things if it > matches. So removing the socket does not hurt as the in-flight skbs are > removed and the receive filters are purged anyway when the socket disappears. > > Signed-off-by: Oliver Hartkopp <socketcan@...tkopp.net> > > --- > > Here's my idea to fix up the original skb->sk handling concept for detecting > the originating socket instance. The patch applies properly down to 3.11. > > Eric please take a look if I used sock_alloc_send_skb() correctly in bcm.c. > Andre please check if you see any functional differences on your machine. I think this is opening yet another can of bugs. CAN calls dev_queue_xmit() : This layer can zap skb->cb[] totally. Please hold a reference on the socket instead. Thats a 10 lines patch, instead of yet another complex patch. void can_destructor(struct sk_buff *skb) { sock_put(skb->sk); } void can_set_owner(struct sk_buff *skb, struct sock *sk) { skb_orphan(skb); skb->destructor = can_skb_destructor; skb->sk = sk; sock_hold(sk); } Thats how every protocol does this, with variants. Check l2tp_sock_wfree()/l2tp_skb_set_owner_w() for an example -- 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