[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4B461EEB.7040806@hartkopp.net>
Date: Thu, 07 Jan 2010 18:50:35 +0100
From: Oliver Hartkopp <oliver@...tkopp.net>
To: David Miller <davem@...emloft.net>
CC: netdev@...r.kernel.org, Wolfgang Grandegger <wg@...ndegger.com>
Subject: Re: [RFC] ndo_validate_skb: Let the netdev check a valid skb content
David Miller wrote:
> From: Oliver Hartkopp <oliver@...tkopp.net>
> Date: Wed, 23 Dec 2009 20:23:27 +0100
>
>> @@ -2034,6 +2035,14 @@ int dev_queue_xmit(struct sk_buff *skb)
>> goto out_kfree_skb;
>> }
>>
>> + /* If the device offers a function to validate the skb content, let
>> + * it check the skb and return an error to the caller if it fails.
>> + */
>> + if (ops->ndo_validate_skb && ops->ndo_validate_skb(skb)) {
>> + rc = -EINVAL;
>> + goto out_kfree_skb;
>> + }
>> +
>
> To me this is as valuable as no error at all and simply having
> the driver drop the frame. Which is what we do now. I can
> sniff at the receiver to see that the device never transmitted
> the frame.
>
> Is this getting this generic -EINVAL back so important that it's
> worth adding the new if() test for the NULL method to every packet
> that traverses the machine? Keep in mind we can route at rates
> exceeding 1 million packets per second, so ever cycle you add
> here really matters.
>
> If you want to place the hooks still, put them out of the fast
> path, which is probably in AF_PACKET. We can extend this to
> call the validation function from other "untrusted" sources.
>
> But do realize that you're not saving anything, things like
> queueing disciplines and traffic classifiers in the qdisc
> layer can modify any part of the packet however they wish.
>
> So even if the CAN protocol layer itself emitted a valid
> frame, the qdisc layer can "corrupt" it.
I see. Thanks for your detailed answer!
Indeed silently drop the invalid CAN frame inside the driver (and increase
dev->stats.tx_dropped) looks like the best solution to me now.
If people try to push rubbish into the device, tx_dropped is an appropriate
indication.
Thanks,
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
Powered by blists - more mailing lists