* Re: [Socketcan-users] [PATCH] CAN: make checking in can_rcv less restrictive [not found] ` <4A7B0957.5020808@hartkopp.net> @ 2009-08-07 6:52 ` Rémi Denis-Courmont 2009-08-07 11:35 ` Oliver Hartkopp 0 siblings, 1 reply; 4+ messages in thread From: Rémi Denis-Courmont @ 2009-08-07 6:52 UTC (permalink / raw) To: ext Oliver Hartkopp, netdev@vger.kernel.org Cc: Luotao Fu, socketcan-users@lists.berlios.de, Michael Olbrich Moving to netdev.... On Thursday 06 August 2009 19:48:23 ext Oliver Hartkopp wrote: > The CAN applications can rely on getting proper CAN frames with this check. > It was introduced some time ago together with several other sanity checks - > even on the TX path. > > The CAN core *only* consumes skbuffs originated from a CAN netdevice > (ARPHRD_CAN). > > When this BUG() triggers, someone provided a definitely broken *CAN* > network driver, and this needs to be fixed on that level. It is really not > that problematic to ensure proper CAN frames on driver level ... this > sanity check should not be needed to be performed by every single > application. AFAIK, the TUN driver can inject layer-2 frames of any type, any size and any content from userspace into the packet type handler. Sure enough, you need CAP_NET_ADMIN and r/w access to /dev/net/tun but is it sufficient to bring the system down? -- Rémi Denis-Courmont Nokia Devices R&D, Maemo Software, Helsinki ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Socketcan-users] [PATCH] CAN: make checking in can_rcv less restrictive 2009-08-07 6:52 ` [Socketcan-users] [PATCH] CAN: make checking in can_rcv less restrictive Rémi Denis-Courmont @ 2009-08-07 11:35 ` Oliver Hartkopp 2009-08-07 11:46 ` Luotao Fu 2009-08-07 11:54 ` Rémi Denis-Courmont 0 siblings, 2 replies; 4+ messages in thread From: Oliver Hartkopp @ 2009-08-07 11:35 UTC (permalink / raw) To: Rémi Denis-Courmont, Luotao Fu Cc: netdev@vger.kernel.org, socketcan-users@lists.berlios.de, Michael Olbrich Rémi Denis-Courmont wrote: > Moving to netdev.... > > On Thursday 06 August 2009 19:48:23 ext Oliver Hartkopp wrote: >> The CAN applications can rely on getting proper CAN frames with this check. >> It was introduced some time ago together with several other sanity checks - >> even on the TX path. >> >> The CAN core *only* consumes skbuffs originated from a CAN netdevice >> (ARPHRD_CAN). >> >> When this BUG() triggers, someone provided a definitely broken *CAN* >> network driver, and this needs to be fixed on that level. It is really not >> that problematic to ensure proper CAN frames on driver level ... this >> sanity check should not be needed to be performed by every single >> application. > > AFAIK, the TUN driver can inject layer-2 frames of any type, any size and any > content from userspace into the packet type handler. Sure enough, you need > CAP_NET_ADMIN and r/w access to /dev/net/tun but is it sufficient to bring the > system down? > The complete code section currently looks like this: static int can_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev) { struct dev_rcv_lists *d; struct can_frame *cf = (struct can_frame *)skb->data; int matches; if (dev->type != ARPHRD_CAN || !net_eq(dev_net(dev), &init_net)) { kfree_skb(skb); return 0; } BUG_ON(skb->len != sizeof(struct can_frame) || cf->can_dlc > 8); (..) So you would need to have an originating interface with ARPHRD_CAN ... Do you think, it's still possible with the TUN driver? @Luotao: I talked to Urs and we discussed to prepare a patch that only creates a warning and drops the skb afterwards, as the problem is not critical for a proper ongoing kernel operation. I think, that was you original intention: if (!net_eq(dev_net(dev), &init_net) || WARN_ON(dev->type != ARPHRD_CAN) || WARN_ON(skb->len != sizeof(struct can_frame) || cf->can_dlc > 8)) { kfree_skb(skb); return NET_RX_BAD; } Would this be ok for you? Regards, Oliver ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Socketcan-users] [PATCH] CAN: make checking in can_rcv less restrictive 2009-08-07 11:35 ` Oliver Hartkopp @ 2009-08-07 11:46 ` Luotao Fu 2009-08-07 11:54 ` Rémi Denis-Courmont 1 sibling, 0 replies; 4+ messages in thread From: Luotao Fu @ 2009-08-07 11:46 UTC (permalink / raw) To: Oliver Hartkopp Cc: R?mi Denis-Courmont, Luotao Fu, netdev@vger.kernel.org, socketcan-users@lists.berlios.de, Michael Olbrich [-- Attachment #1: Type: text/plain, Size: 1024 bytes --] Hi Oliver, On Fri, Aug 07, 2009 at 01:35:26PM +0200, Oliver Hartkopp wrote: > R?mi Denis-Courmont wrote: .... > > @Luotao: I talked to Urs and we discussed to prepare a patch that only creates > a warning and drops the skb afterwards, as the problem is not critical for a > proper ongoing kernel operation. I think, that was you original intention: > > if (!net_eq(dev_net(dev), &init_net) || > WARN_ON(dev->type != ARPHRD_CAN) || > WARN_ON(skb->len != sizeof(struct can_frame) || cf->can_dlc > 8)) { > kfree_skb(skb); > return NET_RX_BAD; > } > > Would this be ok for you? I'm absolutely fine with this. thx cheers Fu -- Pengutronix e.K. | Dipl.-Ing. Luotao Fu | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Socketcan-users] [PATCH] CAN: make checking in can_rcv less restrictive 2009-08-07 11:35 ` Oliver Hartkopp 2009-08-07 11:46 ` Luotao Fu @ 2009-08-07 11:54 ` Rémi Denis-Courmont 1 sibling, 0 replies; 4+ messages in thread From: Rémi Denis-Courmont @ 2009-08-07 11:54 UTC (permalink / raw) To: ext Oliver Hartkopp Cc: netdev@vger.kernel.org, socketcan-users@lists.berlios.de On Friday 07 August 2009 14:35:26 ext Oliver Hartkopp wrote: > So you would need to have an originating interface with ARPHRD_CAN ... > > Do you think, it's still possible with the TUN driver? Hmm, actually no... TUN can create packets with any type (ETH_P_*), but devices always have ARPHRD_NONE type. -- Rémi Denis-Courmont Nokia Devices R&D, Maemo Software, Helsinki ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-08-07 11:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1249572295-7801-1-git-send-email-l.fu@pengutronix.de>
[not found] ` <4A7B0957.5020808@hartkopp.net>
2009-08-07 6:52 ` [Socketcan-users] [PATCH] CAN: make checking in can_rcv less restrictive Rémi Denis-Courmont
2009-08-07 11:35 ` Oliver Hartkopp
2009-08-07 11:46 ` Luotao Fu
2009-08-07 11:54 ` Rémi Denis-Courmont
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).