[PATCH] CTA_PROTO_NUM is u_int8_t not u_int16_t (was Re: CTA_PROTO_NUM u_int8_t or u_int16_t)

[Harald Welte]

Sorry, forget that other patch.

The only way how we can thoroughly solve (and avoid) this problem, also
for future cases, is to behave like 'real' TLV parsers (e.g. ASN1).

That is, for any kind of numeric values, we don't make an assumption
that the attribute has a certain fixed size.  Instead, we derive the
(u8/u16/u32/u64) size from the length of the attribute, i.e.
NFA_PAYLOAD() is 1/2/4/8 bytes long.

This is a quick and dirty patch to demonstrate what I mean:

=====

diff --git a/net/ipv4/netfilter/ip_conntrack_netlink.c b/net/ipv4/netfilter/ip_conntrack_netlink.c
--- a/net/ipv4/netfilter/ip_conntrack_netlink.c
+++ b/net/ipv4/netfilter/ip_conntrack_netlink.c
@@ -502,12 +502,13 @@ ctnetlink_parse_tuple_ip(struct nfattr *
 }
 
 static const size_t cta_min_proto[CTA_PROTO_MAX] = {
-	[CTA_PROTO_NUM-1]	= sizeof(u_int16_t),
+	[CTA_PROTO_NUM-1]	= sizeof(u_int8_t),
 	[CTA_PROTO_SRC_PORT-1]	= sizeof(u_int16_t),
 	[CTA_PROTO_DST_PORT-1]	= sizeof(u_int16_t),
 	[CTA_PROTO_ICMP_TYPE-1]	= sizeof(u_int8_t),
 	[CTA_PROTO_ICMP_CODE-1]	= sizeof(u_int8_t),
 	[CTA_PROTO_ICMP_ID-1]	= sizeof(u_int16_t),
+	[CTA_PROTO-1]		= sizeof(u_int8_t),
 };
 
 static inline int
@@ -527,7 +528,18 @@ ctnetlink_parse_tuple_proto(struct nfatt
 
 	if (!tb[CTA_PROTO_NUM-1])
 		return -EINVAL;
-	tuple->dst.protonum = *(u_int16_t *)NFA_DATA(tb[CTA_PROTO_NUM-1]);
+
+	switch (NFA_PAYLOAD(tb[CTA_PROTO_NUM-1]))
+	case sizeof(u_int8_t):
+		tuple->dst.protonum = 
+				*(u_int8_t *)NFA_DATA(tb[CTA_PROTO_NUM-1]);
+		break;
+	case sizeof(u_int16_t):
+		tuple->dst.protonum = 
+				*(u_int16_t *)NFA_DATA(tb[CTA_PROTO_NUM-1]);
+	default:
+		return -EINVAL;
+	}
 
 	proto = ip_conntrack_proto_find_get(tuple->dst.protonum);

=====

Obviously, this needs to be moved into a nfnetlink core funciton,
something like a function nfattr_parse_number() that would then be
called from all places that parse a number.

Userspace parsers (libnetfilter_conntrack) would have to introduce the
same semantics.

It might be a bit too much overhead, I'm not really decided yet.  But in
the end, if everybody plays according to that rule, we don't have any
such issues in the future.

Comments?

-- 
- Harald Welte <laforge at netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : /pipermail/netfilter-devel/attachments/20051124/784e2bf1/attachment-0001.pgp