[PATCH] complete nfnl_talk

[PATCH] complete nfnl_talk

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 108 bytes --]

Hi Harald,
this patch completes nfnl_talk and removes a couple
of compiler warnings.

Best regards,
Patrick

[-- Attachment #2: libnfnetlink-nfnl_talk.diff --]
[-- Type: text/plain, Size: 3636 bytes --]

? ChangeLog
Index: iptables2/libctnetlink/libctnetlink.c
===================================================================
RCS file: /cvspublic/netfilter/iptables2/libctnetlink/libctnetlink.c,v
retrieving revision 1.3
diff -u -r1.3 libctnetlink.c
--- iptables2/libctnetlink/libctnetlink.c	8 Aug 2002 10:27:46 -0000	1.3
+++ iptables2/libctnetlink/libctnetlink.c	2 Jun 2003 18:37:18 -0000
@@ -34,7 +34,7 @@
 #include "libctnetlink.h"
 
 #define ctnl_error(format, args...) \
-	fprintf(stderr, __FUNCTION__ ": " format, ## args)
+	fprintf(stderr, "%s: " format, __FUNCTION__, ## args)
 
 /***********************************************************************
  * low level stuff 
Index: iptables2/libnfnetlink/libnfnetlink.c
===================================================================
RCS file: /cvspublic/netfilter/iptables2/libnfnetlink/libnfnetlink.c,v
retrieving revision 1.2
diff -u -r1.2 libnfnetlink.c
--- iptables2/libnfnetlink/libnfnetlink.c	2 Aug 2002 08:17:18 -0000	1.2
+++ iptables2/libnfnetlink/libnfnetlink.c	2 Jun 2003 18:37:18 -0000
@@ -21,7 +21,7 @@
 #include "libnfnetlink.h"
 
 #define nfnl_error(format, args...) \
-	fprintf(stderr, __FUNCTION__ ": " format "\n", ## args)
+	fprintf(stderr, "%s: " format "\n", __FUNCTION__, ## args)
 
 #ifdef _NFNL_DEBUG
 #define nfnl_debug_dump_packet nfnl_dump_packet
@@ -35,7 +35,7 @@
 	struct nfattr *nfa = NFM_NFA(NLMSG_DATA(nlh));
 	int len = NFM_PAYLOAD(nlh);
 
-	printf(__FUNCTION__ " called from %s\n", desc);
+	printf("%s called from %s\n", __FUNCTION__, desc);
 	printf("  nlmsghdr = %p, received_len = %u\n", nlh, received_len);
 	printf("  NLMSG_DATA(nlh) = %p (+%u bytes)\n", nlmsg_data,
 	       (nlmsg_data - (void *)nlh));
@@ -250,13 +250,12 @@
 	return 0;
 }
 
-#if 0
 int nfnl_talk(struct nfnl_handle *nfnlh, struct nlmsghdr *n, pid_t peer,
 	      unsigned groups, struct nlmsghdr *answer,
 	      int (*junk)(struct sockaddr_nl *, struct nlmsghdr *n, void *),
 	      void *jarg)
 {
-	char buf[CTNL_BUFFSIZE];
+	char buf[NFNL_BUFFSIZE];
 	struct sockaddr_nl nladdr;
 	struct nlmsghdr *h;
 	unsigned int seq;
@@ -307,15 +306,65 @@
 			return -1;
 		}
 
-		for (h = (struct nlmsghdr *)buf; status >= sizeof(*h)) {
+		for (h = (struct nlmsghdr *)buf; status >= sizeof(*h); ) {
 			int len = h->nlmsg_len;
 			int l = len - sizeof(*h);
 			int err;
 
-				
+			if (l < 0 || len > status) {
+				if (msg.msg_flags & MSG_TRUNC) {
+					nfnl_error("Truncated message\n");
+					return -1;
+				}
+				nfnl_error("Malformed message: len=%d\n", len);
+				return -1; /* FIXME: libnetlink exits here */
+			}
+
+			if (h->nlmsg_pid != nfnlh->local.nl_pid ||
+			    h->nlmsg_seq != seq) {
+				if (junk) {
+					err = junk(&nladdr, h, jarg);
+					if (err < 0)
+						return err;
+				}
+				continue;
+			}
 
+			if (h->nlmsg_type == NLMSG_ERROR) {
+				struct nlmsgerr *err = NLMSG_DATA(h);
+				if (l < sizeof(struct nlmsgerr))
+					nfnl_error("ERROR truncated\n");
+				else {
+					errno = -err->error;
+					if (errno == 0) {
+						if (answer)
+							memcpy(answer, h, h->nlmsg_len);
+						return 0;
+					}
+					perror("CTNETLINK answers");
+				}
+				return -1;
+			}
+			if (answer) {
+				memcpy(answer, h, h->nlmsg_len);
+				return 0;
+			}
+
+			nfnl_error("Unexpected reply!\n");
+
+			status -= NLMSG_ALIGN(len);
+			h = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));
+		}
+		if (msg.msg_flags & MSG_TRUNC) {
+			nfnl_error("Messages truncated\n");
+			continue;
+		}
+		if (status) {
+			nfnl_error("Remnant of size %d\n", status);
+			exit(1);
+		}
+	}
 }
-#endif
 
 /**
  * nfnl_addattr_l - Add variable length attribute to nlmsghdr

Re: [PATCH] complete nfnl_talk

[Martin Josefsson]

On Mon, 2003-06-02 at 20:39, Patrick McHardy wrote:
> Hi Harald,
> this patch completes nfnl_talk and removes a couple
> of compiler warnings.

Hi Patrick

Good thing that you are fixing up this stuff.

I have two patches that might be of interest. They are old and quite
ugly...

http://kashyyyk.netfilter.org/~gandalf/patches/nfnetlink_conntrack-fix.diff
Without this patch it's very easy for a regular user to cause a NULL
pointer dereference. Harald has already included the kfree hunk in cvs
but not the memset's.

http://kashyyyk.netfilter.org/~gandalf/patches/libctnetlink-020809-2.diff
Change the ctnl_get_conntrack() interface a bit (incremental to my
earlier fixes to that stuff).

And finally the patch I use for oidentd
http://kashyyyk.netfilter.org/~gandalf/patches/oidentd-2.0.3+cvs20020603-ctnetlink-diff
Just here so people can see the interface.

-- 
/Martin

Re: [PATCH] complete nfnl_talk

[Patrick McHardy]

Hi Martin,

Martin Josefsson wrote:

>Hi Patrick
>
>Good thing that you are fixing up this stuff.
>
>I have two patches that might be of interest. They are old and quite
>ugly...
>
>http://kashyyyk.netfilter.org/~gandalf/patches/nfnetlink_conntrack-fix.diff
>Without this patch it's very easy for a regular user to cause a NULL
>pointer dereference. Harald has already included the kfree hunk in cvs
>but not the memset's.
>

I don't know if you've seen my changes to nfnetlink, i've added the memsets
to nfnetlink_check_attributes and changed ctnl_dump_table (iirc thats where
the kfree_skb was) quite a bit.

>http://kashyyyk.netfilter.org/~gandalf/patches/libctnetlink-020809-2.diff
>Change the ctnl_get_conntrack() interface a bit (incremental to my
>earlier fixes to that stuff).
>
>And finally the patch I use for oidentd
>http://kashyyyk.netfilter.org/~gandalf/patches/oidentd-2.0.3+cvs20020603-ctnetlink-diff
>Just here so people can see the interface.
>  
>

I have to get the source and patch it, i tried figuring out what exactly it
does from the diffs but didn't got it ;)

Thanks for the hints,
Patrick

Re: [PATCH] complete nfnl_talk

[Patrick McHardy]

One tought about the patch:

> int nfnl_talk(struct nfnl_handle *nfnlh, struct nlmsghdr *n, pid_t peer,
> 	      unsigned groups, struct nlmsghdr *answer,
> 	      int (*junk)(struct sockaddr_nl *, struct nlmsghdr *n, void *),
> 	      void *jarg)
> 
>
...

> 
>+			if (h->nlmsg_type == NLMSG_ERROR) {
>+				struct nlmsgerr *err = NLMSG_DATA(h);
>+				if (l < sizeof(struct nlmsgerr))
>+					nfnl_error("ERROR truncated\n");
>+				else {
>+					errno = -err->error;
>+					if (errno == 0) {
>+						if (answer)
>+							memcpy(answer, h, h->nlmsg_len);
>+						return 0;
>+					}
>+					perror("CTNETLINK answers");
>+				}
>+				return -1;
>  
>

applications might want to handle the error themselves, an alternative 
would be:

errno = -err->error;
if (anwer) {
        memcpy(answer, h, h->nlmsg_len);
        return errno;
} else if (errno)
        perror("CTNETLINK answers);
return errno;

What do you think ?
Patrick