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 0000000..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 = 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(&event_lock); > + return NULL; > +} > + > +/** > + * check_wanted_data() > + * we don't send unwanted informations to userspace, according to the > + * dynamic configuration in the rbtree > + */ > +static int check_wanted_event(struct sock *sk, enum cn_net_socket > syscall_num) { > + int err = -EINVAL; > + struct event ev; > + > + if (!sk) > + return err; > + > + ev.syscall_num = syscall_num; > + ev.protocol = sk->sk_protocol; > + > + /* check if the event is already registered */ > + if (lookup_event(ev)) > + err = 0; > + > + return err; > +} > + > +/** > + * dump_event() > + * dump the entire rbtree in log > + */ > +static void dump_event(void) { > + struct rb_node *p = NULL ; > + struct event_node *tmp = NULL; > + > + read_lock(&event_lock); > + p = rb_first(&event_tree); > + while (p) { > + tmp = rb_entry(p, struct event_node, ev_node); > + printk(KERN_INFO "%d:%s\n", > + tmp->ev.protocol, syscall_name[tmp->ev.syscall_num]); > + p = rb_next(p); > + }; > + read_unlock(&event_lock); > +} I doubt it should exist - it is purely debug stuff. > +/** > + * remove_all_event() > + * delete all events registered in the rbtree > + */ > +static enum ack_err remove_all_event(void) { > + struct rb_node *p = NULL; > + struct event_node *cur = NULL; > + > + write_lock(&event_lock); > + while ((p = rb_first(&event_tree)) != NULL) { > + cur = rb_entry(p, struct event_node, ev_node); > + rb_erase(p, &event_tree); > + kfree(cur); > + } > + write_unlock(&event_lock); > + > + /* rbtree is now empty */ > + return CN_NET_ACK_SUCCES; > +} > + > +/** > + * alloc_event() > + * alloc memory for a event_node, and return pointer to it > + */ > +static struct event_node *alloc_event(void) { > + struct event_node *evn = NULL; > + evn = kzalloc(sizeof(struct event_node), GFP_KERNEL); > + return evn; > +} > + > +/** > + * insert_event() > + * insert a event in the rbtree > + * we are checking if we are trying to register a same event twice > + */ > +static enum ack_err insert_event(struct event ev) { > + struct rb_node **rb_link, *rb_parent; > + struct event_node *new_evn; > + enum ack_err ret = CN_NET_ACK_SUCCES; > + > + rb_link = &event_tree.rb_node; > + rb_parent = NULL; > + > + write_lock(&event_lock); > + > + while(*rb_link) { > + struct event_node *tmp; > + > + rb_parent = *rb_link; > + tmp = rb_entry(rb_parent, struct event_node, ev_node); > + > + if (ev.syscall_num < tmp->ev.syscall_num) > + rb_link = &rb_parent->rb_left; > + else if (ev.syscall_num > tmp->ev.syscall_num) > + rb_link = &rb_parent->rb_right; > + else if (ev.protocol < tmp->ev.protocol) > + rb_link = &rb_parent->rb_left; > + else if (ev.protocol > tmp->ev.protocol) > + rb_link = &rb_parent->rb_right; > + else { > + /* event is already registered */ > + ret = CN_NET_ACK_EINCONFIG; > + goto out; > + } > + } > + > + /* no match: event is added to the tree */ > + if ((new_evn = alloc_event()) != NULL) { This is broken - you allocate under lock with gfp_kernel flags. I would just allocate event in advance and insert it, if insertion fails, event can be freed. > + new_evn->ev.syscall_num = ev.syscall_num; > + new_evn->ev.protocol = ev.protocol; > + } else { > + /* can't allocate memory, exiting */ > + ret = CN_NET_ACK_ENOMEM; > + goto out; > + } > + > + rb_link_node(&new_evn->ev_node, rb_parent, rb_link); > + rb_insert_color(&new_evn->ev_node, &event_tree); > + > +out: > + write_unlock(&event_lock); > + return ret; > +} > + > +/** > + * remove_event() > + * delete a entry from the rbtree > + * we are checking if we are trying to delete a unregistered event > + */ > +static enum ack_err remove_event(struct event ev) { > + struct event_node *cur = NULL; > + enum ack_err ret = CN_NET_ACK_EINCONFIG; > + struct rb_node *rb_node = event_tree.rb_node; > + > + write_lock(&event_lock); > + > + while (rb_node) { > + cur = rb_entry(rb_node, struct event_node, ev_node); > + > + if (is_same_event(cur->ev, ev)) > + break; > + > + 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; > + } > + > + if (rb_node) { > + rb_erase(&cur->ev_node, &event_tree); > + kfree(cur); > + ret = CN_NET_ACK_SUCCES; > + } > + write_unlock(&event_lock); > + > + return ret; > +} > + > +/** > + * do_register() > + * check if userpace protocol version is same as kernel protocol version > + */ > +static enum ack_err do_register(__u32 version) { > + enum ack_err err = CN_NET_ACK_SUCCES; > + > + DEBUGP(KERN_INFO "do_register: %d %d\n", > + version, CN_NET_VERSION); > + > + if (version == CN_NET_VERSION) > + atomic_inc(&net_event_num_listeners); > + else > + err = CN_NET_ACK_EBADPROTO; > + return err; > +} > + > +/** > + * do_config() > + * execute config asked by userspace > + * return enum ack_err > + */ > +static enum ack_err do_config(struct msg_config *cfg) { > + enum ack_err err = CN_NET_ACK_SUCCES; > + > + DEBUGP(KERN_INFO "do_config: %s %s %d\n", > + config_name[cfg->config_cmd], > + syscall_name[cfg->ev.syscall_num], > + cfg->ev.protocol); > + > + switch (cfg->config_cmd) { > + case CN_NET_CONFIG_ADD: > + err = insert_event(cfg->ev); > + break; > + case CN_NET_CONFIG_DEL: > + err= remove_event(cfg->ev); > + break; > + case CN_NET_CONFIG_FLUSH: > + err = remove_all_event(); I would call it 'remove_all_events()'. > + break; > + default: > + err = CN_NET_ACK_EINTYPE; > + break; > + }; > + > + return err; > +} > + > +/** > + * cn_net_ack > + * Send an acknowledgement message to userspace > + * > + * Use 0 for SUCCESS, EFOO otherwise. (see enum ack_err) > + */ > +static void cn_net_ack(enum ack_err err, unsigned int rcvd_seq, unsigned int > rcvd_ack, enum msg_type msg_type) > +{ > + struct cn_msg *m; > + struct net_event *net_ev; > + __u8 buffer[CN_NET_MSG_SIZE]; > + > + DEBUGP(KERN_INFO "cn_net_ack: listen=%d\n", > atomic_read(&net_event_num_listeners)); > + > + if (atomic_read(&net_event_num_listeners) < 1 && err != > CN_NET_ACK_EBADPROTO) > + return; Why don't you like CN_NET_ACK_EBADPROTO? > + m = (struct cn_msg *) buffer; > + net_ev = (struct net_event *) m->data; > + > + net_ev->msg_type = msg_type; > + ktime_get_real_ts(&net_ev->timestamp); Please be absolutely shure you do not have misalignment here and/or different size issues with 32/64 dual arches like x86_64. > + net_ev->net_event_data.ack = err; > + memcpy(&m->id, &cn_net_event_id, sizeof(m->id)); > + m->seq = rcvd_seq; > + m->ack = rcvd_ack + 1; > + m->len = sizeof(struct net_event); > + cn_netlink_send(m, CN_IDX_NET, gfp_any()); > +} > + > +/** > + * cn_net_ctl > + * connector callback > + * @data: message receive from userspace via the connector > + */ > +void cn_net_ctl(void *data) { > + struct cn_msg *m = data; > + struct net_event *net_ev = NULL; > + enum msg_type msg_type; > + enum ack_err err = CN_NET_ACK_SUCCES; > + > + if (m->len != sizeof(struct net_event)) { > + printk(KERN_WARNING "cn_net_ctl : message with bad size, > discard\n"); Either put it under printk_ratelimit() or discard at all. > + return; > + } > + > + net_ev = (struct net_event *) m->data; > + > + if (net_ev->msg_type != CN_NET_LISTEN && > + atomic_read(&net_event_num_listeners) < 1) { > + printk(KERN_WARNING "cn_net_ctl : register first\n"); The same. > + return; > + } > + msg_type = CN_NET_ACK; /* default response message type */ > + > + switch (net_ev->msg_type) { > + case CN_NET_NONE: /* want to play ping pong ? */ > + msg_type = net_ev->msg_type; > + break; > + case CN_NET_ACK: /* userspace is ack'ing - check that */ > + /* FIXME: we don't send an ACK to an ACK */ > + /* we just check which message is ack */ > + goto out; > + break; > + case CN_NET_DATA: /* CN_NET_DATA can't be used by userspace */ > + err = CN_NET_ACK_EINTYPE; > + break; > + case CN_NET_CONFIG: /* configuring kernel's behaviour */ > + err = do_config(&(net_ev->net_event_data.config)); > + break; > + case CN_NET_LISTEN: /* userspace is registering */ > + err = do_register(net_ev->net_event_data.version); > + break; > + case CN_NET_IGNORE: /* userspace is unregistering */ > + atomic_dec(&net_event_num_listeners); > + break; > + case CN_NET_DUMP: /* dumping the rbtree -- debug purpose */ > + dump_event(); > + break; > + default: > + err = CN_NET_ACK_EINTYPE; > + break; > + }; > + cn_net_ack(err, m->seq, m->ack, msg_type); > +out: > + return; > +} > + > +/** > + * cn_net_send_event > + * send data to userspace > + * @sock: sock which sock->sk->sk_state change to the state identified by > @event > + */ > +static void cn_net_send_event(struct sock *sk, struct sockaddr *address, > + enum cn_net_socket syscall_num) { > + > + struct net_event *net_ev; > + struct cn_msg *m; > + struct inet_sock *inet = inet_sk(sk); > + struct sockaddr_in *addr4 = NULL; > + struct sockaddr_in6 *addr6 = NULL; > + struct task_struct *me = current; > + > + __u8 buffer[CN_NET_MSG_SIZE]; > + > + DEBUGP(KERN_INFO "cn_net_ack: listen=%d\n", > atomic_read(&net_event_num_listeners)); > + > + if (atomic_read(&net_event_num_listeners) < 1) > + goto out; > + > + m = (struct cn_msg *) buffer; > + net_ev = (struct net_event *) m->data; > + > + switch (syscall_num) { > + case CN_NET_SOCKET_LISTEN: > + case CN_NET_SOCKET_SHUTDOWN: > + case CN_NET_SK_FREE_SECURITY: > + switch (sk->sk_family) { > + case AF_INET: > + net_ev->net_event_data.data.saddr.ipv4 = inet->saddr; > + net_ev->net_event_data.data.daddr.ipv4 = inet->daddr; > + break; > + case AF_INET6: > + memcpy(&(net_ev->net_event_data.data.saddr.ipv6), > + &(inet->pinet6->saddr), sizeof(struct in6_addr)); > + memcpy(&(net_ev->net_event_data.data.daddr.ipv6), > + &(inet->pinet6->daddr), sizeof(struct in6_addr)); > + break; > + default: > + /* other protocol, sending nothing */ > + goto out; > + break; > + }; > + net_ev->net_event_data.data.sport = ntohs(inet->sport); > + net_ev->net_event_data.data.dport = ntohs(inet->dport); > + break; > + case CN_NET_SOCKET_BIND: > + switch (sk->sk_family) { > + case AF_INET: > + addr4 = (struct sockaddr_in *) address; > + net_ev->net_event_data.data.saddr.ipv4 = > addr4->sin_addr.s_addr; > + net_ev->net_event_data.data.daddr.ipv4 = inet->daddr; > + net_ev->net_event_data.data.sport = > ntohs(addr4->sin_port); > + net_ev->net_event_data.data.dport = ntohs(inet->dport); > + break; > + case AF_INET6: > + addr6 = (struct sockaddr_in6 *) address; > + memcpy(&(net_ev->net_event_data.data.saddr.ipv6), > + &(addr6->sin6_addr), sizeof(struct in6_addr)); > + memcpy(&(net_ev->net_event_data.data.daddr.ipv6), > + &(inet->pinet6->daddr), sizeof(struct in6_addr)); > + net_ev->net_event_data.data.sport = > ntohs(addr6->sin6_port); > + net_ev->net_event_data.data.dport = ntohs(inet->dport); > + break; > + default: > + /* other protocol, sending nothing */ > + goto out; > + break; > + }; > + break; > + case CN_NET_SOCKET_CONNECT: > + switch (sk->sk_family) { > + case AF_INET: > + addr4 = (struct sockaddr_in *) address; > + net_ev->net_event_data.data.saddr.ipv4 = inet->saddr; > + net_ev->net_event_data.data.daddr.ipv4 = > addr4->sin_addr.s_addr; > + net_ev->net_event_data.data.sport = ntohs(inet->sport); > + net_ev->net_event_data.data.dport = > ntohs(addr4->sin_port); > + break; > + case AF_INET6: > + addr6 = (struct sockaddr_in6 *) address; > + memcpy(&(net_ev->net_event_data.data.saddr.ipv6), > + &(inet->pinet6->saddr), sizeof(struct in6_addr)); > + memcpy(&(net_ev->net_event_data.data.daddr.ipv6), > + &(addr6->sin6_addr), sizeof(struct in6_addr)); > + net_ev->net_event_data.data.sport = ntohs(inet->sport); > + net_ev->net_event_data.data.dport = > ntohs(addr6->sin6_port); > + break; > + default: > + /* other protocol, sending nothing */ > + goto out; > + break; > + }; > + break; > + default: > + /* Bad syscall_num */ > + break; > + }; > + net_ev->msg_type = CN_NET_DATA; > + ktime_get_real_ts(&net_ev->timestamp); > + net_ev->net_event_data.data.uid = me->uid; > + get_task_comm(net_ev->net_event_data.data.taskname, me); > + net_ev->net_event_data.data.ev.protocol = sk->sk_protocol; > + net_ev->net_event_data.data.ev.syscall_num = syscall_num; > + net_ev->net_event_data.data.family = sk->sk_family; > + memcpy(&m->id, &cn_net_event_id, sizeof(m->id)); > + m->seq = get_random_int(); You generally want to use linear sequence number, although it only matters if protocol does support its check - you can use ny number of course. > + m->ack = 0; > + m->len = sizeof(struct net_event); > + > + cn_netlink_send(m, CN_IDX_NET, gfp_any()); > +out: > + return; > +} > + > +static int cn_net_socket_listen(struct socket *sock, int backlog) { > + > + DEBUGP(KERN_INFO "cn_net_socket_listen\n"); > + if (check_wanted_event(sock->sk, CN_NET_SOCKET_LISTEN) == NULL) > + cn_net_send_event(sock->sk, NULL, CN_NET_SOCKET_LISTEN); > + > + return 0; > +} > + > +static int cn_net_socket_bind(struct socket *sock, > + struct sockaddr *address, int addrlen) { > + > + DEBUGP(KERN_INFO "cn_net_socket_bind\n"); > + if (check_wanted_event(sock->sk, CN_NET_SOCKET_BIND) == NULL) > + cn_net_send_event(sock->sk, address, CN_NET_SOCKET_BIND); > + > + return 0; > +} > + > +static int cn_net_socket_connect(struct socket *sock, struct sockaddr > *address, int addrlen) { > + > + DEBUGP(KERN_INFO "cn_net_socket_connect\n"); > + if (check_wanted_event(sock->sk, CN_NET_SOCKET_CONNECT) == NULL) > + cn_net_send_event(sock->sk, address, CN_NET_SOCKET_CONNECT); > + > + return 0; > +} > + > +static int cn_net_socket_shutdown(struct socket *sock, int how) { > + > + DEBUGP(KERN_INFO "cn_net_socket_shutdown\n"); > + if (check_wanted_event(sock->sk, CN_NET_SOCKET_SHUTDOWN) == NULL) > + cn_net_send_event(sock->sk, NULL, CN_NET_SOCKET_SHUTDOWN); > + > + return 0; > +} > + > +static int cn_net_sk_free_security(struct sock *sk) { > + > + DEBUGP(KERN_INFO "cn_net_sk_free_security\n"); > + if (check_wanted_event(sk, CN_NET_SK_FREE_SECURITY) == NULL) > + cn_net_send_event(sk, NULL, CN_NET_SK_FREE_SECURITY); > + > + return 0; > +} > + > +static struct security_operations cn_net_security_ops = { > + .socket_listen = cn_net_socket_listen, > + .socket_bind = cn_net_socket_bind, > + .socket_connect = cn_net_socket_connect, > + .socket_shutdown = cn_net_socket_shutdown, > + .sk_free_security = cn_net_sk_free_security, > +}; > + > +static int __init init(void) { > + int err = 0; > + > + err = cn_add_callback(&cn_net_event_id, cn_net_event_name, &cn_net_ctl); > + if (err) { > + printk(KERN_WARNING "cn_net: Failure add connector callback\n"); > + goto out_callback; > + } > + > + if (register_security(&cn_net_security_ops)) { > + printk(KERN_INFO "cn_net: Failure registering with kernel\n"); > + if (mod_reg_security(MY_NAME, &cn_net_security_ops)) { > + printk(KERN_WARNING > + "cn_net: Failure registering with primary > security" > + " module\n"); > + err = -EINVAL; > + goto out_security; > + } > + secondary = 1; > + } > + > + printk(KERN_INFO "cn_net: network events module loaded\n"); > + return 0; > + > +out_security: > + cn_del_callback(&cn_net_event_id); > + > +out_callback: > + return err; > +} > + > +static void __exit fini(void) { > + if (secondary) { > + if (mod_unreg_security(MY_NAME, &cn_net_security_ops)) > + printk(KERN_INFO "cn_net: Failure unregistering with" > + " primary security module\n"); > + } else { > + if (unregister_security(&cn_net_security_ops)) > + printk(KERN_INFO "cn_net: Failure unregistering with " > + "kernel\n"); > + } > + > + cn_del_callback(&cn_net_event_id); > + > + /* clean memory */ > + remove_all_event(); > + > + printk(KERN_INFO "cn_net: network events module unloaded\n"); > +} > + > +module_init(init); > +module_exit(fini); > + > +MODULE_DESCRIPTION("Network events module"); > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Samir Bellabes <[EMAIL PROTECTED]>"); > diff --git a/include/linux/cn_net.h b/include/linux/cn_net.h > new file mode 100644 > index 0000000..0d86715 > --- /dev/null > +++ b/include/linux/cn_net.h > @@ -0,0 +1,122 @@ > +/* > + * include/linux/cn_net.h > + * > + * 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. > + */ > + > +#ifndef CN_NET_H > +#define CN_NET_H > + > +#include <linux/time.h> > +#include <linux/ipv6.h> > + > +#define CN_NET_VERSION 0x1 > + > +#define CN_NET_MSG_SIZE (sizeof(struct cn_msg) + sizeof(struct net_event)) > + > +char *syscall_name[] = { "LISTEN", "BIND", "CONNECT", "SHUTDOWN", "SK_FREE" > }; > +char *msg_type_name[] = { "CN_NET_NONE", "CN_NET_ACK", "CN_NET_DATA", > + "CN_NET_CONFIG", "CN_NET_LISTEN", "CN_NET_IGNORE", > + "CN_NET_DUMP" }; > +char *config_name[] = {"CN_NET_CONFIG_ADD", "CN_NET_CONFIG_DEL", > "CN_NET_CONFIG_FLUSH" }; Why they should live in header file? I would sugest to put them into source and provide appropriate helper functions to access strings. > +/** > + * identify which syscall has been called. > + */ > +enum cn_net_socket { > + CN_NET_SOCKET_LISTEN = 0, > + CN_NET_SOCKET_BIND = 1, > + CN_NET_SOCKET_CONNECT = 2, > + CN_NET_SOCKET_SHUTDOWN = 3, > + CN_NET_SK_FREE_SECURITY = 4, > + CN_NET_SOCKET_MAX = 5, > +}; > + > +/** > + * Protocol message type > + */ > +enum msg_type { > + CN_NET_NONE = 0, > + CN_NET_ACK = 1, > + CN_NET_DATA = 2, > + CN_NET_CONFIG = 3, > + CN_NET_LISTEN = 4, > + CN_NET_IGNORE = 5, > + CN_NET_DUMP = 6, > +}; > + > +/** > + * values for CN_NET_ACK messages > + */ > +enum ack_err { > + CN_NET_ACK_SUCCES = 0, > + CN_NET_ACK_ENOMEM = 1, > + CN_NET_ACK_EINCONFIG = 2, > + CN_NET_ACK_EINTYPE = 3, > + CN_NET_ACK_EBADPROTO = 4, > +}; > + > +/** > + * values for CN_NET_CONFIG messages > + */ > +enum config_cmd { > + CN_NET_CONFIG_ADD = 0, > + CN_NET_CONFIG_DEL = 1, > + CN_NET_CONFIG_FLUSH = 2, > +}; > + > +struct event { > + enum cn_net_socket syscall_num; > + __u8 protocol; > +}; Imho you should not use enum as a type - I would put there two shorts to align to 32 bits. > +struct event_node { > + struct rb_node ev_node; > + struct event ev; > +}; > + > +struct msg_config { > + enum config_cmd config_cmd; > + struct event ev; > +}; > + > +struct net_event { > + enum msg_type msg_type; > + struct timespec timestamp; NO WAY! It has different size on 32 and 64 bit platforms, you are not allowed to use it in code supposed to be seen from userspace. > + union { > + /* protocol version number */ > + __u32 version; > + > + /* generic ack for both userspace and kernel */ > + enum ack_err ack; > + > + /* send data to userspace */ > + struct { > + struct event ev; > + uid_t uid; > + unsigned char taskname[TASK_COMM_LEN]; > + unsigned int family; > + union { > + struct in6_addr ipv6; > + __u32 ipv4; > + } saddr; > + union { > + struct in6_addr ipv6; > + __u32 ipv4; > + } daddr; > + unsigned int sport; > + unsigned int dport; > + } data; > + > + /* send config to kernel */ > + struct msg_config config; > + } net_event_data; > +}; And generally all your names in this header suck :) Add something uniq to them line cn_event_ or ne_ or somthing else, but definitely not 'struct event'. -- 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