Re: [RFC] [PATCH] Network Events Connector
On Thu, Mar 15, 2007 at 02:05:53AM +0100, Samir Bellabes ([EMAIL PROTECTED])
wrote:
Hi,
Hi Samir.
My comments are below.
here a updated patch for the network events connector.
review are welcome :)
Thanks.
[PATCH] Network Events Connector
This patch adds a connector which reports networking's events to
userspace.
It's sending events when a userspace application is using
syscalls so with LSM, we are catching this, at hooks : socket_listen,
socket_bind, socket_connect, socket_shutdown and sk_free_security.
Wanted events are dynamically manage from userspace, in a rbtree. A
daemon, cn_net_daemon, is listening for the connector in userspace.
daemon and doc are available here :
http://people.mandriva.com/~sbellabes/cn_net/
Signed-off-by: Samir Bellabes [EMAIL PROTECTED]
--
drivers/connector/Kconfig |8
drivers/connector/Makefile |1
drivers/connector/cn_net.c | 618
+
include/linux/cn_net.h | 122
include/linux/connector.h |5
5 files changed, 752 insertions(+), 2 deletions(-)
--
diff --git a/drivers/connector/Kconfig b/drivers/connector/Kconfig
index e0bdc0d..0b04952 100644
--- a/drivers/connector/Kconfig
+++ b/drivers/connector/Kconfig
@@ -18,4 +18,12 @@ config PROC_EVENTS
Provide a connector that reports process events to userspace. Send
events such as fork, exec, id change (uid, gid, suid, etc), and exit.
+config NET_EVENTS
+ boolean Report network events to userspace
+ depends on CONNECTOR=y SECURITY_NETWORK
+ default y
+ ---help---
+ Provide a connector that reports networking's events to userspace.
+ Send events such as DCCP/TCP listen/close and UDP bind/close.
+
endmenu
diff --git a/drivers/connector/Makefile b/drivers/connector/Makefile
index 1f255e4..436bb5d 100644
--- a/drivers/connector/Makefile
+++ b/drivers/connector/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_CONNECTOR) += cn.o
obj-$(CONFIG_PROC_EVENTS)+= cn_proc.o
+obj-$(CONFIG_NET_EVENTS) += cn_net.o
cn-y += cn_queue.o connector.o
diff --git a/drivers/connector/cn_net.c b/drivers/connector/cn_net.c
new file mode 100644
index 000..a15f67b
--- /dev/null
+++ b/drivers/connector/cn_net.c
@@ -0,0 +1,618 @@
+/*
+ * drivers/connector/cn_net.c
+ *
+ * Network events connector
+ * Samir Bellabes [EMAIL PROTECTED]
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/skbuff.h
+#include linux/security.h
+#include linux/netlink.h
+#include linux/connector.h
+#include linux/net.h
+#include net/sock.h
+#include net/inet_sock.h
+#include linux/in.h
+#include linux/ipv6.h
+#include linux/in6.h
+#include linux/rbtree.h
+#include linux/list.h
+#include linux/spinlock.h
+#include linux/random.h
+
+#include linux/cn_net.h
+
+static atomic_t net_event_num_listeners = ATOMIC_INIT(0);
+static struct cb_id cn_net_event_id = { CN_IDX_NET, CN_VAL_NET };
+static char cn_net_event_name[] = cn_net_event;
+static int secondary = 0;
+
+struct rb_root event_tree = RB_ROOT;
That should be static.
+static rwlock_t event_lock = RW_LOCK_UNLOCKED;
+
+#if defined(CONFIG_NET_EVENTS)
+#define MY_NAME THIS_MODULE-name
+#else
+#define MY_NAME cn_net
+#endif
+
Why is it needed?
+#if 0
+#define DEBUGP printk
+#else
+#define DEBUGP(format, args...)
+#endif
+
+/**
+ * is_same_event()
+ * check if two events are the same or not
+ */
+static unsigned int is_same_event(struct event one, struct event two) {
+ return ((one.syscall_num == two.syscall_num)
+ (one.protocol == two.protocol));
+}
Coding style is broken, place '{' on the next line.
+/**
+ * lookup_event()
+ * look for a match in the rbtree
+ * returns address of the wanted element if it is in the rbtree, or NULL
+ */
+static struct event_node *lookup_event(struct event ev) {
+ struct rb_node *rb_node = event_tree.rb_node;
+
+ read_lock(event_lock);
+
+ while (rb_node) {
+ struct event_node *cur = rb_entry(rb_node, struct event_node,
ev_node);
+
+ if (is_same_event(cur-ev, ev)) {
+ read_unlock(event_lock);
+ return cur;
+ }
+
+ if (ev.syscall_num cur-ev.syscall_num)
+ rb_node = rb_node-rb_left;
+ else if (ev.syscall_num cur-ev.syscall_num)
+ rb_node =
Re: [RFC] [PATCH] Network Events Connector
Evgeniy Polyakov [EMAIL PROTECTED] writes:
On Fri, Feb 09, 2007 at 05:43:14AM +0100, Samir Bellabes ([EMAIL PROTECTED])
wrote:
Hi,
Here is a new feature which can help firewalls to be more application
aware, so more useful for people.
Our previous discussion about cn_net and firewalls:
http://marc2.theaimsgroup.com/?t=11597695752r=1w=2
Please, I would really like to have feedback and comments on that tool,
in order to improve it.
Technical side does have problems.
1. your way to delete and check events is wrong - there is no need to
allocate new event and search for it in the hash table to remove - use
values as is.
3. why hash table and not rb tree?
You are right, I have rewrite this part of the system, using rbtree
Here is patch to apply on top of the previous 'initialisation path'
patch.
commit 5110880bc67e8329671ffa596cd85c05ec82
Author: Samir Bellabes [EMAIL PROTECTED]
Date: Thu Mar 15 01:42:48 2007 +0100
[PATCH] cn_net: store event's configuration in RB tree
Noticed by Evgeniy Polyakov [EMAIL PROTECTED]
Signed-off-by: Samir Bellabes [EMAIL PROTECTED]
diff --git a/drivers/connector/cn_net.c b/drivers/connector/cn_net.c
index c9eb53e..a15f67b 100644
--- a/drivers/connector/cn_net.c
+++ b/drivers/connector/cn_net.c
@@ -22,7 +22,7 @@ #include net/inet_sock.h
#include linux/in.h
#include linux/ipv6.h
#include linux/in6.h
-#include linux/jhash.h
+#include linux/rbtree.h
#include linux/list.h
#include linux/spinlock.h
#include linux/random.h
@@ -34,10 +34,8 @@ static struct cb_id cn_net_event_id = {
static char cn_net_event_name[] = cn_net_event;
static int secondary = 0;
-static struct list_head *hash = NULL;
-static rwlock_t hash_lock = RW_LOCK_UNLOCKED;
-static int hash_size = 4;
-module_param(hash_size, uint, 0600);
+struct rb_root event_tree = RB_ROOT;
+static rwlock_t event_lock = RW_LOCK_UNLOCKED;
#if defined(CONFIG_NET_EVENTS)
#define MY_NAME THIS_MODULE-name
@@ -61,172 +59,198 @@ static unsigned int is_same_event(struct
}
/**
- * check_event()
- * look for a match in the hash table
- * Returns 1 if data is in the hash table
+ * lookup_event()
+ * look for a match in the rbtree
+ * returns address of the wanted element if it is in the rbtree, or NULL
*/
-static unsigned int check_event(struct event_list *data) {
- unsigned int err = 0, h = 0;
- struct list_head *l;
- struct event_list *evl;
-
- h = jhash((data-ev), sizeof(struct event), 0) % hash_size;
-
- read_lock(hash_lock);
- l = hash[h];
- list_for_each_entry(evl, l, list) {
- if (is_same_event(evl-ev, data-ev)) {
- err = 1;
- break;
+static struct event_node *lookup_event(struct event ev) {
+ struct rb_node *rb_node = event_tree.rb_node;
+
+ read_lock(event_lock);
+
+ while (rb_node) {
+ struct event_node *cur = rb_entry(rb_node, struct event_node,
ev_node);
+
+ if (is_same_event(cur-ev, ev)) {
+ read_unlock(event_lock);
+ return cur;
}
+
+ if (ev.syscall_num cur-ev.syscall_num)
+ rb_node = rb_node-rb_left;
+ else if (ev.syscall_num cur-ev.syscall_num)
+ rb_node = rb_node-rb_right;
+ else if (ev.protocol cur-ev.protocol)
+ rb_node = rb_node-rb_left;
+ else if (ev.protocol cur-ev.protocol)
+ rb_node = rb_node-rb_right;
}
- read_unlock(hash_lock);
- return err;
+
+ read_unlock(event_lock);
+ return NULL;
}
-/**
- * check_wanted_data
- * We don't send unwanted informations to userspace, according to the
- * dynamic configuration in the hash table.
- * @sock: sock which is changing its state
- * Returns: 1 if data have to be send to userspace
+/**
+ * check_wanted_data()
+ * we don't send unwanted informations to userspace, according to the
+ * dynamic configuration in the rbtree
*/
-static int check_wanted_data(struct sock *sk, enum cn_net_socket syscall_num) {
- struct event_list *data = NULL;
- unsigned int err = 0;
-
- if (!sk)
- return -EINVAL;
+static int check_wanted_event(struct sock *sk, enum cn_net_socket syscall_num)
{
+ int err = -EINVAL;
+ struct event ev;
- data = kzalloc(sizeof(struct event_list), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
- data-ev.syscall_num = syscall_num;
- data-ev.protocol = sk-sk_protocol;
- INIT_LIST_HEAD((data-list));
+ if (!sk)
+ return err;
+ ev.syscall_num = syscall_num;
+ ev.protocol = sk-sk_protocol;
+
/* check if the event is already registered */
- err = check_event(data);
- kfree(data);
+ if (lookup_event(ev))
+ err = 0;
+
Re: [RFC] [PATCH] Network Events Connector
Evgeniy Polyakov [EMAIL PROTECTED] writes: On Fri, Feb 09, 2007 at 05:43:14AM +0100, Samir Bellabes ([EMAIL PROTECTED]) wrote: Hi, Here is a new feature which can help firewalls to be more application aware, so more useful for people. Our previous discussion about cn_net and firewalls: http://marc2.theaimsgroup.com/?t=11597695752r=1w=2 Please, I would really like to have feedback and comments on that tool, in order to improve it. Technical side does have problems. 4. are you 100% sure there are misalignments and 32/64 bit userspace problems? you seems to copy some bits from proc connector, which suffered from that errors in the past. No, i'm currently not sure of that. I think you are refering to this : http://git.kernel.org/?p=linux/kernel/git/acme/net-2.6.22.git;a=commitdiff;h=af3e095a1fb42bac32355d5d59ce93f8b4e59a3e I'm going to care of this for cn_net Evgeniy, thanks a lot for your review. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH] Network Events Connector
From: Samir Bellabes [EMAIL PROTECTED]
Date: Thu, 15 Mar 2007 02:05:53 +0100
+#if 0
+#define DEBUGP printk
+#else
+#define DEBUGP(format, args...)
+#endif
Please no local debugging macros.
+static unsigned int is_same_event(struct event one, struct event two) {
Please format functions properly, especially wrt. braces, they don't
belong on the line holding the argument list closing parenthesis:
return_type func_name(args)
{
}
I'm not going to read this any further, please master
Documentation/CodingStyle and resubmit if you want further
review from me.
Thanks.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH] Network Events Connector
On Fri, Feb 09, 2007 at 05:43:14AM +0100, Samir Bellabes ([EMAIL PROTECTED]) wrote: Hi, Here is a new feature which can help firewalls to be more application aware, so more useful for people. Our previous discussion about cn_net and firewalls: http://marc2.theaimsgroup.com/?t=11597695752r=1w=2 Please, I would really like to have feedback and comments on that tool, in order to improve it. Technical side does have problems. 1. your way to delete and check events is wrong - there is no need to allocate new event and search for it in the hash table to remove - use values as is. 2. initialization path has problems - hash is allocated after securty hooks and connector moduler are registered. 3. why hash table and not rb tree? 4. are you 100% sure there are misalignments and 32/64 bit userspace problems? you seems to copy some bits from proc connector, which suffered from that errors in the past. Thanks a lot, Samir Bellabes -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
