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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 02 Jun 2007 21:58:31 +0200
From:	Oliver Hartkopp <>
To:	Patrick McHardy <>,
	Urs Thuermann <>
CC:	David Miller <>,
	Thomas Gleixner <>,
	Oliver Hartkopp <>,
	Urs Thuermann <>,
Subject: Re: [patch 5/7] CAN: Add virtual CAN netdevice driver

The more i was thinking about my own suggested solution the more it
turned out to be ugly for me ...

Summarizing we have two problems to solve:

1. Identify the originating sk to potentially trash my own sent messages.
2. Indicate a requested local loopback to the lower (driver) layer.

Regarding point 1.:
skb->sk is intentionally set to NULL, when ever skb_orphan() or
skb_clone() is invoked to cut the reference to the sk. Performing a
loopback this is a reasonable thing to do as also skb->destructor(skb)
is called in skb_orphan().

Indeed skb->sk is completely unused in the rx path, so we just would
have to 'preserve' skb->sk the way 'up' whenever we make use of
skb_orphan() or skb_clone().

E.g. in af_can.c the deliver() function would be changed like this:

static inline void deliver(struct sk_buff *skb, struct receiver *r)
+	struct sock *sk = skb->sk;
	struct sk_buff *clone = skb_clone(skb, GFP_ATOMIC);

	DBG("skbuff %p cloned to %p\n", skb, clone);
	if (clone) {
+		clone->sk = sk;
		r->func(clone, r->data);

So there is a proper way to make the originating sk avaliable when the
skb reaches the receiving socket.

Regarding point 2.:
To indicate an outgoing skb to be looped back it would just be enough to
set skb->tstamp at skb creation time. This would be similar to set the
timestamp at netdev receive time. skb->tstamp is also completely unused
in the tx path. In the case of local loopback setting the timestamp at
this creation stage is not even a wrong semantic behaviour.

Assuming some clarifying comments in the source code this looks like a
simple to use and clearly arranged implementation to me. I'll make a
test implementation tomorrow to see how if it smells as good as i
expected ;-)

Best  regards,

Oliver Hartkopp wrote:
> Patrick McHardy wrote:
>> Oliver Hartkopp wrote:
>>> Patrick McHardy wrote:
>> Yes, its working, but only in certain combinations and you're breaking
>> the rules for skb->cb, making it impossible for other layers to use.
>> skb->sk is "stable" at the output path, the regular loopback device
>> orphans the skb in hard_start_xmit. So you can at least use it there.
>>> Would therefore skb->cb left unchanged in my skb's? Or is there any flag
>>> that can be set in the skb to keep the packet scheduler's hands off?
>> No, and I don't think we want a flag to signal that something is
>> violating the rules for skb->cb, there are other users of this
>> besides qdiscs.
> Hm - regarding Patricks and Urs' last mails i just had the idea to put
> the sk-reference that's needed for this special
> CAN-only-loopback-functionality into the data section of the skb, e.g.
> by introducing a new struct can_skb_data:
> struct can_skb_data {
>     struct can_frame cf;
>     sock *txsk;
> };
> So instead of allocating the space of struct can_frame the alloc_skb()
> would allocate the size of struct can_skb_data.
> The needed txsk would be stable in any case and could be used like the
> currently missused skb->cb. This would also lead to a type proof(!)
> implementation.
> In raw_rcv() in raw.c there could be a check for the size of struct
> can_skb_data first before checking the txsk - this would also guarantee
> the backward compatibility for current CAN drivers that allocate only
> the size of struct can_frame. For me this looks like a safe and
> compatible (Kernel & CAN) solution.
> Any objections/comments for this approach?
> Best regards,
> Oliver
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to
> More majordomo info at

To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
More majordomo info at

Powered by blists - more mailing lists