netdev - Re: [patch 5/7] CAN: Add virtual CAN netdevice driver

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:	Wed, 30 May 2007 21:48:22 +0200
From:	Oliver Hartkopp <oliver@...tkopp.net>
To:	Patrick McHardy <kaber@...sh.net>
CC:	Urs Thuermann <urs@...ogud.escape.de>,
	David Miller <davem@...emloft.net>,
	Thomas Gleixner <tglx@...utronix.de>,
	Oliver Hartkopp <oliver.hartkopp@...kswagen.de>,
	Urs Thuermann <urs.thuermann@...kswagen.de>,
	netdev@...r.kernel.org
Subject: Re: [patch 5/7] CAN: Add virtual CAN netdevice driver

Patrick McHardy wrote:
>
>>> (..)
>>> Qdiscs might change skb->cb. Maybe use skb->sk?
>>>
>>>       
>> The loopback functionality in CAN is a bit tricky (maybe you can take a
>> look into the Documentation patch [7/7] at chapter 3.2 and 4.1.4).
>>
>> The problem is, that we need a per socket(!) option that enables the
>> loopback for the sent CAN-Frames or not, so the information about
>> loopback a skb content or not is contained inside the skb sent down to
>> the CAN netdevice. If the CAN networkdevice is not capable to perform a
>> loopback itself (see dev->flags, chapter 6) the CAN core has to do the
>> loopback as a fallback.
>>     
>
>
> skb->sk should work fine for a per socket option. In fact the
> CAN core patch simply sets skb->cb to skb->sk.
>
>   
>> The use of skb->cb is also needed in the receive path to recognize and
>> trash loopback'ed CAN-frames inside the *originating* socket (see
>> chapter 4.1.4).
>>     
>
>
> skb->sk should work here as well since you detect these frames
> before queueing to the receiving socket.
>   

Hm - this would indeed be much nicer than using skb->cb.

I think we just had some concerns to use skb->sk for our own
functionality, as it might be used and dereferenced by other layers
between the socket and the network driver ... so setting skb->sk to NULL
(to indicate 'no loopback') smells bad if you don't really know who ever
dereferences skb->sk.

Btw. it's really worth to look at it again. I just had the idea to
access the loopback flag via skb->sk->opt->loopback (argpfl!) on the TX
path and skb->sk on the RX path. This would avoid skb->sk to be set to
NULL but need to have this loopback flag to be implemented in each CAN
socket opt.

I'll discuss this with Urs tomorrow morning, which change would be the
best and also the 'safest' solution here.

Thanks for your contribution!

Oliver

-
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