netdev - Re: [RFC] ndo_validate_skb: Let the netdev check a valid skb content

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]

Message-Id: <20100107.004101.217821816.davem@davemloft.net>
Date:	Thu, 07 Jan 2010 00:41:01 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	oliver@...tkopp.net
Cc:	netdev@...r.kernel.org
Subject: Re: [RFC] ndo_validate_skb: Let the netdev check a valid skb
 content

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.
--
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