All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] LSM networking: skb hooks for 2.5.42 (2/7)
@ 2002-10-15 14:36 James Morris
  2002-10-15 17:40 ` David S. Miller
  0 siblings, 1 reply; 29+ messages in thread
From: James Morris @ 2002-10-15 14:36 UTC (permalink / raw)
  To: David S. Miller, kuznet; +Cc: netdev, linux-security-module

(note: we'd like to use the existing skb security field, but have not 
touched yet it as you may want to maintain the padding there).


diff -urN -X dontdiff linux-2.5.42.w0/include/linux/security.h linux-2.5.42.w1/include/linux/security.h
--- linux-2.5.42.w0/include/linux/security.h	Tue Oct 15 20:28:55 2002
+++ linux-2.5.42.w1/include/linux/security.h	Tue Oct 15 20:25:40 2002
@@ -630,6 +630,50 @@
  * to use nonblocking allocation.
  * 
  *
+ * Lifecycle hooks for network buffers.
+ *
+ * @skb_alloc_security:
+ *	This hook is called by the &sk_buff allocator when a new buffer is
+ *	being allocated.  An LSM module may allocate and assign a new security
+ *	blob for the &sk_buff via this hook.
+ *	@skb contains the buffer being allocated.
+ *	@gfp_mask contains the kernel allocation gfp_mask value.
+ *	Return 0 if successful, or -ENOMEM on out of memory condition.
+ * @skb_clone:
+ *	This hook is called when an &sk_buff is being cloned, and may be used,
+ *	for example, to increment a reference count on the associated security
+ *	blob.  The security blob in the @newskb will not have been allocated.
+ *	@newskb contains the newly cloned buffer.
+ *	@oldskb contains the buffer being cloned.
+ *	Returns 0 on success -ENOMEM on failure.
+ * @skb_copy:
+ *	This hook is called when an &sk_buff header is being copied, which
+ *	occurs during the skb_copy() and pskb_copy() functions in
+ *	<net/core/skbuff.c>
+ *	@newskb contains the newly copied buffer.
+ *	@oldskb contains the buffer being copied.
+ * @skb_set_owner_w:
+ *	This hook is called when the ownership of an &sk_buff is being assigned
+ *	to a sending socket.  Typically, this would be used to copy security
+ *	attributes from the sending socket to the &sk_buff.
+ *	@skb contains the buffer being owned.
+ *	@sk contains sock to which ownership is being assigned.
+ * @skb_recv_datagram:
+ *      This hook is called when a process is receiving a datagram
+ *      message.  At this point, there is an association between the
+ *      current process, the socket, and the skb.
+ *      @skb contains the buffer being returned.
+ *      @sk is the receiving sock.
+ *      @flags contains operational flags.
+ * @skb_free_security:
+ *	This hook is called when an &sk_buff is being destroyed, and should be
+ *	used to free any associated security blob.
+ *	@skb contains the buffer being destroyed.
+ *
+ * These are the lifecycle hooks for network buffers. They are used to help
+ * manage the lifecycle of security blobs for &sk_buff structures, and are not
+ * intended to be used for access decisions.
+ *
  * @ptrace:
  *	Check permission before allowing the @parent process to trace the
  *	@child process.
@@ -846,6 +890,16 @@
 
 	void (*netdev_unregister) (struct net_device * dev);
 	
+	int (*skb_alloc_security) (struct sk_buff * skb, int gfp_mask);
+	int (*skb_clone) (struct sk_buff * newskb,
+	                  const struct sk_buff * oldskb);
+	void (*skb_copy) (struct sk_buff * newskb,
+	                  const struct sk_buff * oldskb);
+	void (*skb_set_owner_w) (struct sk_buff * skb, struct sock * sk);
+	void (*skb_recv_datagram) (struct sk_buff * skb, struct sock * sk,
+	                           unsigned flags);
+	void (*skb_free_security) (struct sk_buff * skb);
+	
 	int (*ipc_permission) (struct kern_ipc_perm * ipcp, short flag);
 
 	int (*msg_queue_alloc_security) (struct msg_queue * msq);
diff -urN -X dontdiff linux-2.5.42.w0/include/linux/skbuff.h linux-2.5.42.w1/include/linux/skbuff.h
--- linux-2.5.42.w0/include/linux/skbuff.h	Sun Sep  1 11:34:46 2002
+++ linux-2.5.42.w1/include/linux/skbuff.h	Tue Oct 15 20:23:42 2002
@@ -245,6 +245,8 @@
 #ifdef CONFIG_NET_SCHED
        __u32			tc_index;               /* traffic control index */
 #endif
+
+	void		*lsm_security;		/* replaces the above security field */
 };
 
 #define SK_WMEM_MAX	65535
diff -urN -X dontdiff linux-2.5.42.w0/include/net/sock.h linux-2.5.42.w1/include/net/sock.h
--- linux-2.5.42.w0/include/net/sock.h	Sat Oct 12 15:09:43 2002
+++ linux-2.5.42.w1/include/net/sock.h	Tue Oct 15 20:23:42 2002
@@ -663,6 +663,7 @@
 	skb->sk = sk;
 	skb->destructor = sock_wfree;
 	atomic_add(skb->truesize, &sk->wmem_alloc);
+	security_ops->skb_set_owner_w(skb, sk);
 }
 
 static inline void skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
diff -urN -X dontdiff linux-2.5.42.w0/net/core/datagram.c linux-2.5.42.w1/net/core/datagram.c
--- linux-2.5.42.w0/net/core/datagram.c	Sun Aug 11 12:20:40 2002
+++ linux-2.5.42.w1/net/core/datagram.c	Tue Oct 15 20:23:42 2002
@@ -176,8 +176,10 @@
 		} else
 			skb = skb_dequeue(&sk->receive_queue);
 
-		if (skb)
+		if (skb) {
+			security_ops->skb_recv_datagram(skb, sk, flags);
 			return skb;
+		}
 
 		/* User doesn't want to wait */
 		error = -EAGAIN;
diff -urN -X dontdiff linux-2.5.42.w0/net/core/skbuff.c linux-2.5.42.w1/net/core/skbuff.c
--- linux-2.5.42.w0/net/core/skbuff.c	Sun Sep  1 11:34:46 2002
+++ linux-2.5.42.w1/net/core/skbuff.c	Tue Oct 15 20:23:42 2002
@@ -53,6 +53,7 @@
 #include <linux/rtnetlink.h>
 #include <linux/init.h>
 #include <linux/highmem.h>
+#include <linux/security.h>
 
 #include <net/protocol.h>
 #include <net/dst.h>
@@ -194,6 +195,11 @@
 	if (!data)
 		goto nodata;
 
+	if (security_ops->skb_alloc_security(skb, gfp_mask)) {
+ 		kfree(data);
+		goto nodata;
+	}
+
 	/* XXX: does not include slab overhead */
 	skb->truesize = size + sizeof(struct sk_buff);
 
@@ -252,6 +258,7 @@
 #ifdef CONFIG_NET_SCHED
 	skb->tc_index	  = 0;
 #endif
+	skb->lsm_security = NULL;
 }
 
 static void skb_drop_fraglist(struct sk_buff *skb)
@@ -328,6 +335,7 @@
 #ifdef CONFIG_NETFILTER
 	nf_conntrack_put(skb->nfct);
 #endif
+	security_ops->skb_free_security(skb);
 	skb_headerinit(skb, NULL, 0);  /* clean state */
 	kfree_skbmem(skb);
 }
@@ -355,6 +363,11 @@
 		if (!n)
 			return NULL;
 	}
+	
+	if (security_ops->skb_clone(n, skb)) {
+		skb_head_to_pool(n);
+		return NULL;
+	}
 
 #define C(x) n->x = skb->x
 
@@ -442,6 +455,7 @@
 #ifdef CONFIG_NET_SCHED
 	new->tc_index	= old->tc_index;
 #endif
+	security_ops->skb_copy(new, old);
 }
 
 /**
diff -urN -X dontdiff linux-2.5.42.w0/security/capability.c linux-2.5.42.w1/security/capability.c
--- linux-2.5.42.w0/security/capability.c	Tue Oct 15 20:28:55 2002
+++ linux-2.5.42.w1/security/capability.c	Tue Oct 15 20:23:42 2002
@@ -719,6 +719,37 @@
 	return;
 }
 
+static int cap_skb_alloc_security (struct sk_buff *skb, int gfp_mask)
+{
+	return 0;
+}
+
+static int cap_skb_clone (struct sk_buff *newskb, const struct sk_buff *oldskb)
+{
+	return 0;
+}
+
+static void cap_skb_copy (struct sk_buff *newskb, const struct sk_buff *oldskb)
+{
+	return;
+}
+
+static void cap_skb_set_owner_w (struct sk_buff *skb, struct sock *sk)
+{
+	return;
+}
+
+static void cap_skb_recv_datagram (struct sk_buff *skb, struct sock *sk,
+				   unsigned flags)
+{
+	return;
+}
+
+static void cap_skb_free_security (struct sk_buff *skb)
+{
+	return;
+}
+
 static int cap_register (const char *name, struct security_operations *ops)
 {
 	return -EINVAL;
@@ -822,6 +853,13 @@
 	.task_kmod_set_label =		cap_task_kmod_set_label,
 	.task_reparent_to_init =	cap_task_reparent_to_init,
 
+	.skb_alloc_security =		cap_skb_alloc_security,
+	.skb_clone =			cap_skb_clone,
+	.skb_copy =			cap_skb_copy,
+	.skb_set_owner_w =		cap_skb_set_owner_w,
+	.skb_recv_datagram =		cap_skb_recv_datagram,
+	.skb_free_security =		cap_skb_free_security,
+
 	.ipc_permission =		cap_ipc_permission,
 
 	.msg_queue_alloc_security =	cap_msg_queue_alloc_security,
diff -urN -X dontdiff linux-2.5.42.w0/security/dummy.c linux-2.5.42.w1/security/dummy.c
--- linux-2.5.42.w0/security/dummy.c	Tue Oct 15 20:28:55 2002
+++ linux-2.5.42.w1/security/dummy.c	Tue Oct 15 20:23:42 2002
@@ -534,6 +534,39 @@
 	return;
 }
 
+static int dummy_skb_alloc_security (struct sk_buff *skb, int gfp_mask)
+{
+	return 0;
+}
+
+static int dummy_skb_clone (struct sk_buff *newskb,
+			     const struct sk_buff *oldskb)
+{
+	return 0;
+}
+
+static void dummy_skb_copy (struct sk_buff *newskb,
+			    const struct sk_buff *oldskb)
+{
+	return;
+}
+
+static void dummy_skb_set_owner_w (struct sk_buff *skb, struct sock *sk)
+{
+	return;
+}
+
+static void dummy_skb_recv_datagram (struct sk_buff *skb, struct sock *sk,
+				     unsigned flags)
+{
+	return;
+}
+
+static void dummy_skb_free_security (struct sk_buff *skb)
+{
+	return;
+}
+
 static int dummy_register (const char *name, struct security_operations *ops)
 {
 	return -EINVAL;
@@ -637,6 +670,13 @@
 	.task_kmod_set_label =		dummy_task_kmod_set_label,
 	.task_reparent_to_init =	dummy_task_reparent_to_init,
 
+	.skb_alloc_security =		dummy_skb_alloc_security,
+	.skb_clone =			dummy_skb_clone,
+	.skb_copy =			dummy_skb_copy,
+	.skb_set_owner_w =		dummy_skb_set_owner_w,
+	.skb_recv_datagram =		dummy_skb_recv_datagram,
+	.skb_free_security =		dummy_skb_free_security,
+
 	.ipc_permission =		dummy_ipc_permission,
 	
 	.msg_queue_alloc_security =	dummy_msg_queue_alloc_security,

^ permalink raw reply	[flat|nested] 29+ messages in thread
* (no subject)
@ 2002-10-17  7:41 Rusty Russell
  2002-10-17 17:20 ` [RFC] change format of LSM hooks Daniel Phillips
  2002-10-17 17:25 ` Daniel Phillips
  0 siblings, 2 replies; 29+ messages in thread
From: Rusty Russell @ 2002-10-17  7:41 UTC (permalink / raw)
  To: Daniel Phillips, S; +Cc: Roman Zippel, linux-kernel

In message <E181zuY-0004Fl-00@starship> you write:
> On Thursday 17 October 2002 00:48, Rusty Russell wrote:
> > > On Wednesday 16 October 2002 08:11, Rusty Russell wrote:
> > > > It needs to be turned off when dealing with any interface which might
> > > > be used by one of the hard modules.  Which is pretty bad.
> > > 
> > > As far as I can see, preemption only has to be disabled during the 
> > > synchronize_kernel phase of unloading that one module, and this requireme
nt 
> > > is inherited neither by dependant or depending modules.
> > 
> > No, someone could already have been preempted before you start
> > synchronize_kernel().
> 
> I don't get that.  The sequence is:
> 
>   - turn off preemption
>   - unhook call points
>   - synchronize_kernel
>   - ...
> 
> which doesn't leave any preemption hole that I can see, so I can't comment
> on a couple of the other points until you clear that one up.

You mean that "turn off preemption" also wakes up anyone currently
preempted?  Otherwise they're preempted just inside one of those call
points.

> > Still a race between the zero check and the can't-increment state
> > setting.
> 
> But that one is easy: the zero check just takes the same spinlock as 
> TRY_INC_MOD_COUNT, then sets can't-increment only in the case the count
> is zero, considerably simpler than:

The current spinlock is horrible.  You could use a brlock, of course,
but I didn't mainly because of code bloat and speed.  My current code
looks like:

static inline int try_module_get(struct module *module)
{
	int ret = 1;

	if (module) {
		unsigned int cpu = get_cpu();
		if (likely(module->ref[cpu].live))
			local_inc(&module->ref[cpu].counter);
		else
			ret = 0;
		put_cpu();
	}
	return ret;
}

Which is small enough to be inlined quite nicely, and very fast.
Adding br_read_lock_irqsave() starts to get big and slow (at that
point it's more likely we want to move the module case out of line).

> > This is what my current code does: rmmod itself checks (if
> > /proc/modules available), then the kernel sets the module to
> > can't-increment, then checks again.  If the non-blocking flag is set,
> > it then re-animates the module and fails, otherwise it waits.
> 
> and leaves no window for spurious failure.  The still-initializing case is
> also easy, e.g., a filesystem module simply doesn't call register_filesystem
> until it's completely ready to service calls, so nobody is able to do
> TRY_INC_MOD_COUNT.

Consider some code which needs to know when cpus go up and down, so
registers a notifier.  If the notifier fires before the init is
finished, the notifier code will fail to "try_inc_mod_count()" and
won't call it (it doesn't do try_inc_mod_count at the moment, but
that's a bug).

I don't know of any code which does this now, but it is at least a
theoretical problem.

> > BTW, current patchset (2.5.43):
> 
> Thanks, I'll read them all on the 21st ;-)  The other thing I need to read
> closely is Roman's strategy for changing the module format, and the weird
> linker connections.

Roman dislikes linking in the kernel.  So did I until I wrote it: it's
really trivial (esp. compared with the code to coordinate with the
userspace linker properly).  And it exists today.  The linking takes
around 200 lines.  But, let's say his solution is 500 lines shorter
than mine.

For those five hundred lines, the new parameter infrastructure and
module versioning changes can be done *without* requiring any changes
in modutils.  If you've been following the module changes closely in
the last couple of years, you'll realize what a pain it has been to
introduce changes like licensing, etc.  This frees up our hand.

IMHO, the benifits of having it in-kernel outweigh the slight extra
size.

> > ...The second is the "die-mother-fucker-die"
> > version, which taints the kernel and just removes the damn thing.  For
> > most people, this is better than a reboot, and will usually "work".
> 
> Is there a case where removing a module would actually help?  What is
> the user going to do next, try to reinsert the same module?

Insert a fixed one, hopefully 8).  I was thinking for kernel
developers, and general robustness (eg. an oops inside a module leaves
its refcount at 1).

> > http://www.kernel.org/pub/linux/kernel/people/rusty/patches/Module/force-un
load.patch.gz
> 
> ERROR 404: Not Found.

Damn my fingers.  Updated (now applies on top of the others) but I
haven't tested this version yet (that's what I'm doing now):

http://www.kernel.org/pub/linux/kernel/people/rusty/patches/Module/forceunload.patch.gz

Cheers!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

^ permalink raw reply	[flat|nested] 29+ messages in thread
end of thread, other threads:[~2002-10-19  3:29 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-10-15 14:36 [PATCH] LSM networking: skb hooks for 2.5.42 (2/7) James Morris
2002-10-15 17:40 ` David S. Miller
2002-10-15 18:14   ` Donald Becker
2002-10-15 19:16     ` Greg KH
2002-10-15 19:34       ` David S. Miller
2002-10-15 19:45         ` Greg KH
2002-10-15 19:45           ` David S. Miller
2002-10-15 20:12             ` Greg KH
2002-10-15 20:10               ` David S. Miller
2002-10-15 20:28                 ` Greg KH
2002-10-15 20:24                   ` David S. Miller
2002-10-16  0:07                   ` [RFC] change format of LSM hooks Greg KH
2002-10-16  0:03                     ` David S. Miller
2002-10-16  8:15                     ` Greg KH
2002-10-16 18:59                       ` Greg KH
2002-10-16 16:33                         ` joe perches
2002-10-16 23:46                           ` Greg KH
2002-10-16 23:56                             ` David S. Miller
2002-10-16 19:07                         ` Greg KH
2002-10-17  1:41                     ` Rusty Russell
2002-10-17  3:33                       ` Daniel Phillips
2002-10-17 13:21                     ` Christoph Hellwig
2002-10-17 16:55                       ` Greg KH
2002-10-19  2:33                 ` [PATCH] LSM networking: skb hooks for 2.5.42 (2/7) Keith Owens
2002-10-19  2:54                   ` Keith Owens
2002-10-19  3:29                     ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2002-10-17  7:41 Rusty Russell
2002-10-17 17:20 ` [RFC] change format of LSM hooks Daniel Phillips
2002-10-18  2:04   ` Rusty Russell
2002-10-17 17:25 ` Daniel Phillips

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.