Re: Tapset for TCP

[Josh Stone <jistone at redhat dot com>]

Breno Leitao wrote:
> Guys, 
>
> I created a tapset for TCP incoming packets, and I hope it's helpful to the
> project.

I'm happy to see such work, and I hope we can find a way to reconcile 
this with the tcp tapset we already have.  Please don't be afraid to rip 
up and modify the existing stuff if you see possible improvements...

I mentioned to you on #systemtap that your embedded-C functions need to 
properly protect against bad pointers.  Here's an example of how to do 
this, as a general demo to everyone:

The original:
> /* Check if the skb is using TCP/IP */
> function istcp:long (data:long)
> %{
> 	struct iphdr *ip;	
> 	struct sk_buff *skb;
> 	int tmp = 0;
>
> 	skb = (struct sk_buff *) THIS->data;
>
> 	if (skb->protocol == htons(ETH_P_IP)){
> 		ip = (struct iphdr *) skb->data;
> 		tmp = (ip->protocol == htons(IPPROTO_TCP));
> 	}
> 	THIS->__retvalue = tmp;
> %}

Using kread() lets you have protection within embedded-C.  The code is 
almost the same, just wrapping each pointer dereference:

function istcp:long (data:long)
%{
	struct iphdr *ip;	
	struct sk_buff *skb;
	int tmp = 0;

	skb = (struct sk_buff *) THIS->data;

	if (kread(&skb->protocol) == htons(ETH_P_IP)){
		ip = (struct iphdr *) kread(&skb->data);
		tmp = (kread(&ip->protocol) == htons(IPPROTO_TCP));
	}
	THIS->__retvalue = tmp;
	CATCH_DEREF_FAULT();
%}


Using @cast lets you avoid embedded-C altogether:

global ETH_P_IP, IPPROTO_TCP // define elsewhere
function istcp2:long (skb:long)
{
	tmp = 0
	if (@cast(skb, "sk_buff")->protocol == htons(ETH_P_IP)) {
		ip = @cast(skb, "sk_buff")->data
		tmp = (@cast(ip, "iphdr")->protocol ==
			htons(IPPROTO_TCP))
	}
	return tmp
}


I'll leave it to you to decide which is easier, but at least with the 
latter you can let stap worry about the safety issues for you.


Josh