All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [LSM] Rework LSM hooks
@ 2004-08-10  8:57 Kurt Garloff
  2004-08-10 13:29 ` James Morris
  2004-08-10 14:16 ` James Morris
  0 siblings, 2 replies; 26+ messages in thread
From: Kurt Garloff @ 2004-08-10  8:57 UTC (permalink / raw)
  To: Linux kernel list; +Cc: James Morris, Chris Wright


[-- Attachment #1.1: Type: text/plain, Size: 2911 bytes --]

Hi,

while looking into LSM hooks and SElinux for SLES9, we came across
a couple of issues and the whole thing ended up in a patch that I
think should be applied upstream.

Some boundary conditions:
* We don't want to use selinux by default: The complexity to set up a
  secure system using it is currently quite complex
* The impact of SElinux on performance on SMP is disastrous
* SElinux needs to be compiled in
* CONFIG_SECURITY should remain on -- it allows the flexibility to
  use different LSMs
* This however has the nasty side-effect of ending up with a system
  that uses dummy rather than the Linux default capabilities; so your
  boot up scripts need to take care of loading it. Making capability
  static is no option either, of course, as you want to be able to
  use a different primary LSM or load capability as secondary LSM.
* Even with selinux=0 and capability loaded, the kernel takes a 
  few percents in networking benchmarks (measured by HP on ia64); 
  this is caused by the slowliness of indirect jumps on ia64.

The first patch patch does just change the selinux default; so you
need to enable with selinux=1.

The second patch is much more involved.
* It changes the hooks in security.h to present the CONFIG_SECURITY
  vs. non CONFIG_SECURITY alongside each other; this way the programmer 
  can not easily get these out of sync, as that would become optically 
   offending.
* It uses a macro and only calls into the LSM if a module is loaded
  if(unlikely(security_enabled)) and otherwise calls into the capability
  code just like if CONFIG_SECURITY was off
* This changes the default to be capabilty, not dummy!
  So no LSM needs to be loaded to offer Linux defaults.
* However, dummy still needs to be compiled in as it serves as default
  implementaion for non-defined routines in the loaded LSM.
  Actually, I would like to get rid of it, but did not want to
  change existing behaviour with this patch.
  (Probably one should review existing LSMs: They should be pretty
   much complete and falling back to capability for the few missing
   functions may be fine -- sometimes dummy and capability are even
   the same ...)
* capability should be compiled a module; so it can be loaded as
  secondary LSM; it can also still be loaded as primary LSM; the only 
  effect of that is a minimal slowdown as the indirect jump would be
  taken ...
* Some implementation details are documented in the code; I avoid 
  LSM unload races without using a barrier (or locking) by setting 
  security_ops to capability_security_ops, e.g. 

Both patches are against 2.6.8-rc3.
And, yes, they're pretty well tested, more exactly the versions
against 2.6.5 ;-)

Please comment / apply,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG / Novell, Nuernberg, DE               Director SUSE Labs

[-- Attachment #1.2: selinux-default-disable --]
[-- Type: text/plain, Size: 535 bytes --]

garloff@suse.de

Performance fix

With selinux enabled, performance sucks, as avc_lock spinlock is taken
in the fast path. #39439

--- linux-2.6.5.x86/security/selinux/hooks.c.orig	2004-05-03 15:15:48.000000000 +0200
+++ linux-2.6.5.x86/security/selinux/hooks.c	2004-05-10 15:14:38.264941199 +0200
@@ -82,7 +82,7 @@ __setup("enforcing=", enforcing_setup);
 #endif
 
 #ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
-int selinux_enabled = 1;
+int selinux_enabled = 0;
 
 static int __init selinux_enabled_setup(char *str)
 {

[-- Attachment #1.3: security-disabled-optimize-cap-default --]
[-- Type: text/plain, Size: 41424 bytes --]

garloff@suse.de

Performance & Cleanup

After idea from Brian Haley (HP):
Don't do indirect calls if security_enabled is off.
It is switched on if a security module registers. 
We don't need to register one any longer to get capabilities.
The commoncaps is always compiled in now, the capability module should
be compiled as a module, so it can be registered as secondary module
(on top of SElinux, e.g.) again. It can be loaded as primary module,
but this won't change anything except rendering performance somewhat
worse.

Index: linux-2.6.7/include/linux/security.h
===================================================================
--- linux-2.6.7.orig/include/linux/security.h	2004-08-05 14:58:22.439493894 +0200
+++ linux-2.6.7/include/linux/security.h	2004-08-05 15:23:42.374099009 +0200
@@ -1232,11 +1232,21 @@
 
 /* global variables */
 extern struct security_operations *security_ops;
+extern int security_enabled;
+
+/* Condition for selinux security_ops invocation */
+#define COND_SECURITY(seop, def)		\
+	(unlikely(security_enabled))? security_ops->seop: def
+
+/* SELinux noop */
+static inline void __selinux_nop(void) {}
+#define SE_NOP __selinux_nop() 
 
 /* inline stuff */
 static inline int security_ptrace (struct task_struct * parent, struct task_struct * child)
 {
-	return security_ops->ptrace (parent, child);
+	return COND_SECURITY(ptrace (parent, child), 
+			 cap_ptrace (parent, child));
 }
 
 static inline int security_capget (struct task_struct *target,
@@ -1244,7 +1263,8 @@
 				   kernel_cap_t *inheritable,
 				   kernel_cap_t *permitted)
 {
-	return security_ops->capget (target, effective, inheritable, permitted);
+	return COND_SECURITY(capget (target, effective, inheritable, permitted),
+			 cap_capget (target, effective, inheritable, permitted));
 }
 
 static inline int security_capset_check (struct task_struct *target,
@@ -1252,7 +1272,8 @@
 					 kernel_cap_t *inheritable,
 					 kernel_cap_t *permitted)
 {
-	return security_ops->capset_check (target, effective, inheritable, permitted);
+	return COND_SECURITY(capset_check (target, effective, inheritable, permitted), 
+			 cap_capset_check (target, effective, inheritable, permitted));
 }
 
 static inline void security_capset_set (struct task_struct *target,
@@ -1260,240 +1281,282 @@
 					kernel_cap_t *inheritable,
 					kernel_cap_t *permitted)
 {
-	security_ops->capset_set (target, effective, inheritable, permitted);
+	COND_SECURITY(capset_set (target, effective, inheritable, permitted), 
+		  cap_capset_set (target, effective, inheritable, permitted));
 }
 
 static inline int security_acct (struct file *file)
 {
-	return security_ops->acct (file);
+	return COND_SECURITY(acct (file), 
+			 0);
 }
 
 static inline int security_sysctl(ctl_table * table, int op)
 {
-	return security_ops->sysctl(table, op);
+	return COND_SECURITY(sysctl(table, op), 
+			 0);
 }
 
 static inline int security_quotactl (int cmds, int type, int id,
 				     struct super_block *sb)
 {
-	return security_ops->quotactl (cmds, type, id, sb);
+	return COND_SECURITY(quotactl (cmds, type, id, sb), 
+			 0);
 }
 
 static inline int security_quota_on (struct file * file)
 {
-	return security_ops->quota_on (file);
+	return COND_SECURITY(quota_on (file), 
+			 0);
 }
 
 static inline int security_syslog(int type)
 {
-	return security_ops->syslog(type);
+	return COND_SECURITY(syslog(type), 
+			 cap_syslog(type));
 }
 
 static inline int security_vm_enough_memory(long pages)
 {
-	return security_ops->vm_enough_memory(pages);
+	return COND_SECURITY(vm_enough_memory(pages),
+			 cap_vm_enough_memory(pages));
 }
 
 static inline int security_bprm_alloc (struct linux_binprm *bprm)
 {
-	return security_ops->bprm_alloc_security (bprm);
+	return COND_SECURITY(bprm_alloc_security (bprm), 
+			 0);
 }
 static inline void security_bprm_free (struct linux_binprm *bprm)
 {
-	security_ops->bprm_free_security (bprm);
+	COND_SECURITY(bprm_free_security (bprm), 
+		  SE_NOP);
 }
 static inline void security_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 {
-	security_ops->bprm_apply_creds (bprm, unsafe);
+	COND_SECURITY(bprm_apply_creds (bprm, unsafe), 
+		  cap_bprm_apply_creds (bprm, unsafe));
 }
 static inline int security_bprm_set (struct linux_binprm *bprm)
 {
-	return security_ops->bprm_set_security (bprm);
+	return COND_SECURITY(bprm_set_security (bprm),
+			 cap_bprm_set_security (bprm));
 }
 
 static inline int security_bprm_check (struct linux_binprm *bprm)
 {
-	return security_ops->bprm_check_security (bprm);
+	return COND_SECURITY(bprm_check_security (bprm), 
+			 0);
 }
 
 static inline int security_bprm_secureexec (struct linux_binprm *bprm)
 {
-	return security_ops->bprm_secureexec (bprm);
+	return COND_SECURITY(bprm_secureexec (bprm),
+			 cap_bprm_secureexec (bprm));
 }
 
 static inline int security_sb_alloc (struct super_block *sb)
 {
-	return security_ops->sb_alloc_security (sb);
+	return COND_SECURITY(sb_alloc_security (sb), 
+			 0);
 }
 
 static inline void security_sb_free (struct super_block *sb)
 {
-	security_ops->sb_free_security (sb);
+	COND_SECURITY(sb_free_security (sb), 
+		  SE_NOP);
 }
 
 static inline int security_sb_copy_data (struct file_system_type *type,
 					 void *orig, void *copy)
 {
-	return security_ops->sb_copy_data (type, orig, copy);
+	return COND_SECURITY(sb_copy_data (type, orig, copy), 
+			 0);
 }
 
 static inline int security_sb_kern_mount (struct super_block *sb, void *data)
 {
-	return security_ops->sb_kern_mount (sb, data);
+	return COND_SECURITY(sb_kern_mount (sb, data), 
+			 0);
 }
 
 static inline int security_sb_statfs (struct super_block *sb)
 {
-	return security_ops->sb_statfs (sb);
+	return COND_SECURITY(sb_statfs (sb), 
+			 0);
 }
 
 static inline int security_sb_mount (char *dev_name, struct nameidata *nd,
 				    char *type, unsigned long flags,
 				    void *data)
 {
-	return security_ops->sb_mount (dev_name, nd, type, flags, data);
+	return COND_SECURITY(sb_mount (dev_name, nd, type, flags, data), 
+			 0);
 }
 
 static inline int security_sb_check_sb (struct vfsmount *mnt,
 					struct nameidata *nd)
 {
-	return security_ops->sb_check_sb (mnt, nd);
+	return COND_SECURITY(sb_check_sb (mnt, nd), 
+			 0);
 }
 
 static inline int security_sb_umount (struct vfsmount *mnt, int flags)
 {
-	return security_ops->sb_umount (mnt, flags);
+	return COND_SECURITY(sb_umount (mnt, flags), 
+			 0);
 }
 
 static inline void security_sb_umount_close (struct vfsmount *mnt)
 {
-	security_ops->sb_umount_close (mnt);
+	COND_SECURITY(sb_umount_close (mnt), 
+		  SE_NOP);
 }
 
 static inline void security_sb_umount_busy (struct vfsmount *mnt)
 {
-	security_ops->sb_umount_busy (mnt);
+	COND_SECURITY(sb_umount_busy (mnt), 
+		  SE_NOP);
 }
 
 static inline void security_sb_post_remount (struct vfsmount *mnt,
 					     unsigned long flags, void *data)
 {
-	security_ops->sb_post_remount (mnt, flags, data);
+	COND_SECURITY(sb_post_remount (mnt, flags, data), 
+		  SE_NOP);
 }
 
 static inline void security_sb_post_mountroot (void)
 {
-	security_ops->sb_post_mountroot ();
+	COND_SECURITY(sb_post_mountroot (), 
+		  SE_NOP);
 }
 
 static inline void security_sb_post_addmount (struct vfsmount *mnt,
 					      struct nameidata *mountpoint_nd)
 {
-	security_ops->sb_post_addmount (mnt, mountpoint_nd);
+	COND_SECURITY(sb_post_addmount (mnt, mountpoint_nd), 
+		  SE_NOP);
 }
 
 static inline int security_sb_pivotroot (struct nameidata *old_nd,
 					 struct nameidata *new_nd)
 {
-	return security_ops->sb_pivotroot (old_nd, new_nd);
+	return COND_SECURITY(sb_pivotroot (old_nd, new_nd), 
+			 0);
 }
 
 static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
 					       struct nameidata *new_nd)
 {
-	security_ops->sb_post_pivotroot (old_nd, new_nd);
+	COND_SECURITY(sb_post_pivotroot (old_nd, new_nd),
+		  SE_NOP);
 }
 
 static inline int security_inode_alloc (struct inode *inode)
 {
-	return security_ops->inode_alloc_security (inode);
+	return COND_SECURITY(inode_alloc_security (inode), 
+			 0);
 }
 
 static inline void security_inode_free (struct inode *inode)
 {
-	security_ops->inode_free_security (inode);
+	COND_SECURITY(inode_free_security (inode),
+		  SE_NOP);
 }
 	
 static inline int security_inode_create (struct inode *dir,
 					 struct dentry *dentry,
 					 int mode)
 {
-	return security_ops->inode_create (dir, dentry, mode);
+	return COND_SECURITY(inode_create (dir, dentry, mode), 
+ 			 0);
 }
 
 static inline void security_inode_post_create (struct inode *dir,
 					       struct dentry *dentry,
 					       int mode)
 {
-	security_ops->inode_post_create (dir, dentry, mode);
+	COND_SECURITY(inode_post_create (dir, dentry, mode),
+ 		  SE_NOP);
 }
 
 static inline int security_inode_link (struct dentry *old_dentry,
 				       struct inode *dir,
 				       struct dentry *new_dentry)
 {
-	return security_ops->inode_link (old_dentry, dir, new_dentry);
+	return COND_SECURITY(inode_link (old_dentry, dir, new_dentry), 
+ 			 0);
 }
 
 static inline void security_inode_post_link (struct dentry *old_dentry,
 					     struct inode *dir,
 					     struct dentry *new_dentry)
 {
-	security_ops->inode_post_link (old_dentry, dir, new_dentry);
+	COND_SECURITY(inode_post_link (old_dentry, dir, new_dentry),
+ 		  SE_NOP);
 }
 
 static inline int security_inode_unlink (struct inode *dir,
 					 struct dentry *dentry)
 {
-	return security_ops->inode_unlink (dir, dentry);
+	return COND_SECURITY(inode_unlink (dir, dentry), 
+ 			 0);
 }
 
 static inline int security_inode_symlink (struct inode *dir,
 					  struct dentry *dentry,
 					  const char *old_name)
 {
-	return security_ops->inode_symlink (dir, dentry, old_name);
+	return COND_SECURITY(inode_symlink (dir, dentry, old_name), 
+ 			 0);
 }
 
 static inline void security_inode_post_symlink (struct inode *dir,
 						struct dentry *dentry,
 						const char *old_name)
 {
-	security_ops->inode_post_symlink (dir, dentry, old_name);
+	COND_SECURITY(inode_post_symlink (dir, dentry, old_name),
+ 		  SE_NOP);
 }
 
 static inline int security_inode_mkdir (struct inode *dir,
 					struct dentry *dentry,
 					int mode)
 {
-	return security_ops->inode_mkdir (dir, dentry, mode);
+	return COND_SECURITY(inode_mkdir (dir, dentry, mode), 
+ 			 0);
 }
 
 static inline void security_inode_post_mkdir (struct inode *dir,
 					      struct dentry *dentry,
 					      int mode)
 {
-	security_ops->inode_post_mkdir (dir, dentry, mode);
+	COND_SECURITY(inode_post_mkdir (dir, dentry, mode),
+ 		  SE_NOP);
 }
 
 static inline int security_inode_rmdir (struct inode *dir,
 					struct dentry *dentry)
 {
-	return security_ops->inode_rmdir (dir, dentry);
+	return COND_SECURITY(inode_rmdir (dir, dentry), 
+ 			 0);
 }
 
 static inline int security_inode_mknod (struct inode *dir,
 					struct dentry *dentry,
 					int mode, dev_t dev)
 {
-	return security_ops->inode_mknod (dir, dentry, mode, dev);
+	return COND_SECURITY(inode_mknod (dir, dentry, mode, dev), 
+ 			 0);
 }
 
 static inline void security_inode_post_mknod (struct inode *dir,
 					      struct dentry *dentry,
 					      int mode, dev_t dev)
 {
-	security_ops->inode_post_mknod (dir, dentry, mode, dev);
+	COND_SECURITY(inode_post_mknod (dir, dentry, mode, dev),
+ 		  SE_NOP);
 }
 
 static inline int security_inode_rename (struct inode *old_dir,
@@ -1501,8 +1564,9 @@
 					 struct inode *new_dir,
 					 struct dentry *new_dentry)
 {
-	return security_ops->inode_rename (old_dir, old_dentry,
-					   new_dir, new_dentry);
+	return COND_SECURITY(inode_rename (old_dir, old_dentry,
+					   new_dir, new_dentry), 
+			 0);
 }
 
 static inline void security_inode_post_rename (struct inode *old_dir,
@@ -1510,232 +1574,274 @@
 					       struct inode *new_dir,
 					       struct dentry *new_dentry)
 {
-	security_ops->inode_post_rename (old_dir, old_dentry,
-						new_dir, new_dentry);
+	COND_SECURITY(inode_post_rename (old_dir, old_dentry,
+						new_dir, new_dentry),
+		  SE_NOP);
 }
 
 static inline int security_inode_readlink (struct dentry *dentry)
 {
-	return security_ops->inode_readlink (dentry);
+	return COND_SECURITY(inode_readlink (dentry), 
+			 0);
 }
 
 static inline int security_inode_follow_link (struct dentry *dentry,
 					      struct nameidata *nd)
 {
-	return security_ops->inode_follow_link (dentry, nd);
+	return COND_SECURITY(inode_follow_link (dentry, nd), 
+			 0);
 }
 
 static inline int security_inode_permission (struct inode *inode, int mask,
 					     struct nameidata *nd)
 {
-	return security_ops->inode_permission (inode, mask, nd);
+	return COND_SECURITY(inode_permission (inode, mask, nd), 
+			 0);
 }
 
 static inline int security_inode_setattr (struct dentry *dentry,
 					  struct iattr *attr)
 {
-	return security_ops->inode_setattr (dentry, attr);
+	return COND_SECURITY(inode_setattr (dentry, attr), 
+			 0);
 }
 
 static inline int security_inode_getattr (struct vfsmount *mnt,
 					  struct dentry *dentry)
 {
-	return security_ops->inode_getattr (mnt, dentry);
+	return COND_SECURITY(inode_getattr (mnt, dentry), 
+			 0);
 }
 
 static inline void security_inode_delete (struct inode *inode)
 {
-	security_ops->inode_delete (inode);
+	COND_SECURITY(inode_delete (inode),
+		  SE_NOP);
 }
 
 static inline int security_inode_setxattr (struct dentry *dentry, char *name,
 					   void *value, size_t size, int flags)
 {
-	return security_ops->inode_setxattr (dentry, name, value, size, flags);
+	return COND_SECURITY(inode_setxattr (dentry, name, value, size, flags),
+			 cap_inode_setxattr (dentry, name, value, size, flags));
 }
 
 static inline void security_inode_post_setxattr (struct dentry *dentry, char *name,
 						void *value, size_t size, int flags)
 {
-	security_ops->inode_post_setxattr (dentry, name, value, size, flags);
+	COND_SECURITY(inode_post_setxattr (dentry, name, value, size, flags),
+		  SE_NOP);
 }
 
 static inline int security_inode_getxattr (struct dentry *dentry, char *name)
 {
-	return security_ops->inode_getxattr (dentry, name);
+	return COND_SECURITY(inode_getxattr (dentry, name), 
+			 0);
 }
 
 static inline int security_inode_listxattr (struct dentry *dentry)
 {
-	return security_ops->inode_listxattr (dentry);
+	return COND_SECURITY(inode_listxattr (dentry), 
+			 0);
 }
 
 static inline int security_inode_removexattr (struct dentry *dentry, char *name)
 {
-	return security_ops->inode_removexattr (dentry, name);
+	return COND_SECURITY(inode_removexattr (dentry, name),
+			 cap_inode_removexattr (dentry, name));
 }
 
 static inline int security_inode_getsecurity(struct dentry *dentry, const char *name, void *buffer, size_t size)
 {
-	return security_ops->inode_getsecurity(dentry, name, buffer, size);
+	return COND_SECURITY(inode_getsecurity(dentry, name, buffer, size), 
+			 -EOPNOTSUPP);
 }
 
 static inline int security_inode_setsecurity(struct dentry *dentry, const char *name, const void *value, size_t size, int flags) 
 {
-	return security_ops->inode_setsecurity(dentry, name, value, size, flags);
+	return COND_SECURITY(inode_setsecurity(dentry, name, value, size, flags), 
+			 -EOPNOTSUPP);
 }
 
 static inline int security_inode_listsecurity(struct dentry *dentry, char *buffer)
 {
-	return security_ops->inode_listsecurity(dentry, buffer);
+	return COND_SECURITY(inode_listsecurity(dentry, buffer), 
+			 0);
 }
 
 static inline int security_file_permission (struct file *file, int mask)
 {
-	return security_ops->file_permission (file, mask);
+	return COND_SECURITY(file_permission (file, mask), 
+			 0);
 }
 
 static inline int security_file_alloc (struct file *file)
 {
-	return security_ops->file_alloc_security (file);
+	return COND_SECURITY(file_alloc_security (file), 
+			 0);
 }
 
 static inline void security_file_free (struct file *file)
 {
-	security_ops->file_free_security (file);
+	COND_SECURITY(file_free_security (file),
+		  SE_NOP);
 }
 
 static inline int security_file_ioctl (struct file *file, unsigned int cmd,
 				       unsigned long arg)
 {
-	return security_ops->file_ioctl (file, cmd, arg);
+	return COND_SECURITY(file_ioctl (file, cmd, arg), 
+			 0);
 }
 
 static inline int security_file_mmap (struct file *file, unsigned long prot,
 				      unsigned long flags)
 {
-	return security_ops->file_mmap (file, prot, flags);
+	return COND_SECURITY(file_mmap (file, prot, flags), 
+			 0);
 }
 
 static inline int security_file_mprotect (struct vm_area_struct *vma,
 					  unsigned long prot)
 {
-	return security_ops->file_mprotect (vma, prot);
+	return COND_SECURITY(file_mprotect (vma, prot), 
+			 0);
 }
 
 static inline int security_file_lock (struct file *file, unsigned int cmd)
 {
-	return security_ops->file_lock (file, cmd);
+	return COND_SECURITY(file_lock (file, cmd), 
+			 0);
 }
 
 static inline int security_file_fcntl (struct file *file, unsigned int cmd,
 				       unsigned long arg)
 {
-	return security_ops->file_fcntl (file, cmd, arg);
+	return COND_SECURITY(file_fcntl (file, cmd, arg), 
+			 0);
 }
 
 static inline int security_file_set_fowner (struct file *file)
 {
-	return security_ops->file_set_fowner (file);
+	return COND_SECURITY(file_set_fowner (file), 
+			 0);
 }
 
 static inline int security_file_send_sigiotask (struct task_struct *tsk,
 						struct fown_struct *fown,
 						int fd, int reason)
 {
-	return security_ops->file_send_sigiotask (tsk, fown, fd, reason);
+	return COND_SECURITY(file_send_sigiotask (tsk, fown, fd, reason), 
+			 0);
 }
 
 static inline int security_file_receive (struct file *file)
 {
-	return security_ops->file_receive (file);
+	return COND_SECURITY(file_receive (file), 
+			 0);
 }
 
 static inline int security_task_create (unsigned long clone_flags)
 {
-	return security_ops->task_create (clone_flags);
+	return COND_SECURITY(task_create (clone_flags), 
+			 0);
 }
 
 static inline int security_task_alloc (struct task_struct *p)
 {
-	return security_ops->task_alloc_security (p);
+	return COND_SECURITY(task_alloc_security (p), 
+			 0);
 }
 
 static inline void security_task_free (struct task_struct *p)
 {
-	security_ops->task_free_security (p);
+	COND_SECURITY(task_free_security (p),
+		  SE_NOP);
 }
 
 static inline int security_task_setuid (uid_t id0, uid_t id1, uid_t id2,
 					int flags)
 {
-	return security_ops->task_setuid (id0, id1, id2, flags);
+	return COND_SECURITY(task_setuid (id0, id1, id2, flags), 
+			 0);
 }
 
 static inline int security_task_post_setuid (uid_t old_ruid, uid_t old_euid,
 					     uid_t old_suid, int flags)
 {
-	return security_ops->task_post_setuid (old_ruid, old_euid, old_suid, flags);
+	return COND_SECURITY(task_post_setuid (old_ruid, old_euid, old_suid, flags),
+			 cap_task_post_setuid (old_ruid, old_euid, old_suid, flags));
 }
 
 static inline int security_task_setgid (gid_t id0, gid_t id1, gid_t id2,
 					int flags)
 {
-	return security_ops->task_setgid (id0, id1, id2, flags);
+	return COND_SECURITY(task_setgid (id0, id1, id2, flags), 
+			 0);
 }
 
 static inline int security_task_setpgid (struct task_struct *p, pid_t pgid)
 {
-	return security_ops->task_setpgid (p, pgid);
+	return COND_SECURITY(task_setpgid (p, pgid), 
+			 0);
 }
 
 static inline int security_task_getpgid (struct task_struct *p)
 {
-	return security_ops->task_getpgid (p);
+	return COND_SECURITY(task_getpgid (p), 
+			 0);
 }
 
 static inline int security_task_getsid (struct task_struct *p)
 {
-	return security_ops->task_getsid (p);
+	return COND_SECURITY(task_getsid (p), 
+			 0);
 }
 
 static inline int security_task_setgroups (struct group_info *group_info)
 {
-	return security_ops->task_setgroups (group_info);
+	return COND_SECURITY(task_setgroups (group_info), 
+			 0);
 }
 
 static inline int security_task_setnice (struct task_struct *p, int nice)
 {
-	return security_ops->task_setnice (p, nice);
+	return COND_SECURITY(task_setnice (p, nice), 
+			 0);
 }
 
 static inline int security_task_setrlimit (unsigned int resource,
 					   struct rlimit *new_rlim)
 {
-	return security_ops->task_setrlimit (resource, new_rlim);
+	return COND_SECURITY(task_setrlimit (resource, new_rlim), 
+			 0);
 }
 
 static inline int security_task_setscheduler (struct task_struct *p,
 					      int policy,
 					      struct sched_param *lp)
 {
-	return security_ops->task_setscheduler (p, policy, lp);
+	return COND_SECURITY(task_setscheduler (p, policy, lp), 
+			 0);
 }
 
 static inline int security_task_getscheduler (struct task_struct *p)
 {
-	return security_ops->task_getscheduler (p);
+	return COND_SECURITY(task_getscheduler (p), 
+			 0);
 }
 
 static inline int security_task_kill (struct task_struct *p,
 				      struct siginfo *info, int sig)
 {
-	return security_ops->task_kill (p, info, sig);
+	return COND_SECURITY(task_kill (p, info, sig), 
+			 0);
 }
 
 static inline int security_task_wait (struct task_struct *p)
 {
-	return security_ops->task_wait (p);
+	return COND_SECURITY(task_wait (p), 
+			 0);
 }
 
 static inline int security_task_prctl (int option, unsigned long arg2,
@@ -1743,60 +1849,71 @@
 				       unsigned long arg4,
 				       unsigned long arg5)
 {
-	return security_ops->task_prctl (option, arg2, arg3, arg4, arg5);
+	return COND_SECURITY(task_prctl (option, arg2, arg3, arg4, arg5), 
+			 0);
 }
 
 static inline void security_task_reparent_to_init (struct task_struct *p)
 {
-	security_ops->task_reparent_to_init (p);
+	COND_SECURITY(task_reparent_to_init (p), 
+		  cap_task_reparent_to_init (p));
 }
 
 static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
 {
-	security_ops->task_to_inode(p, inode);
+	COND_SECURITY(task_to_inode(p, inode),
+		  SE_NOP);
 }
 
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
-	return security_ops->ipc_permission (ipcp, flag);
+	return COND_SECURITY(ipc_permission (ipcp, flag), 
+			 0);
 }
 
 static inline int security_msg_msg_alloc (struct msg_msg * msg)
 {
-	return security_ops->msg_msg_alloc_security (msg);
+	return COND_SECURITY(msg_msg_alloc_security (msg), 
+			 0);
 }
 
 static inline void security_msg_msg_free (struct msg_msg * msg)
 {
-	security_ops->msg_msg_free_security(msg);
+	COND_SECURITY(msg_msg_free_security(msg),
+		  SE_NOP);
 }
 
 static inline int security_msg_queue_alloc (struct msg_queue *msq)
 {
-	return security_ops->msg_queue_alloc_security (msq);
+	return COND_SECURITY(msg_queue_alloc_security (msq), 
+			 0);
 }
 
 static inline void security_msg_queue_free (struct msg_queue *msq)
 {
-	security_ops->msg_queue_free_security (msq);
+	COND_SECURITY(msg_queue_free_security (msq),
+		  SE_NOP);
 }
 
 static inline int security_msg_queue_associate (struct msg_queue * msq, 
 						int msqflg)
 {
-	return security_ops->msg_queue_associate (msq, msqflg);
+	return COND_SECURITY(msg_queue_associate (msq, msqflg), 
+			 0);
 }
 
 static inline int security_msg_queue_msgctl (struct msg_queue * msq, int cmd)
 {
-	return security_ops->msg_queue_msgctl (msq, cmd);
+	return COND_SECURITY(msg_queue_msgctl (msq, cmd), 
+			 0);
 }
 
 static inline int security_msg_queue_msgsnd (struct msg_queue * msq,
 					     struct msg_msg * msg, int msqflg)
 {
-	return security_ops->msg_queue_msgsnd (msq, msg, msqflg);
+	return COND_SECURITY(msg_queue_msgsnd (msq, msg, msqflg), 
+			 0);
 }
 
 static inline int security_msg_queue_msgrcv (struct msg_queue * msq,
@@ -1804,86 +1921,102 @@
 					     struct task_struct * target,
 					     long type, int mode)
 {
-	return security_ops->msg_queue_msgrcv (msq, msg, target, type, mode);
+	return COND_SECURITY(msg_queue_msgrcv (msq, msg, target, type, mode), 
+ 			 0);
 }
 
 static inline int security_shm_alloc (struct shmid_kernel *shp)
 {
-	return security_ops->shm_alloc_security (shp);
+	return COND_SECURITY(shm_alloc_security (shp), 
+ 			 0);
 }
 
 static inline void security_shm_free (struct shmid_kernel *shp)
 {
-	security_ops->shm_free_security (shp);
+	COND_SECURITY(shm_free_security (shp),
+ 		  SE_NOP);
 }
 
 static inline int security_shm_associate (struct shmid_kernel * shp, 
 					  int shmflg)
 {
-	return security_ops->shm_associate(shp, shmflg);
+	return COND_SECURITY(shm_associate(shp, shmflg), 
+ 			 0);
 }
 
 static inline int security_shm_shmctl (struct shmid_kernel * shp, int cmd)
 {
-	return security_ops->shm_shmctl (shp, cmd);
+	return COND_SECURITY(shm_shmctl (shp, cmd), 
+ 			 0);
 }
 
 static inline int security_shm_shmat (struct shmid_kernel * shp, 
 				      char __user *shmaddr, int shmflg)
 {
-	return security_ops->shm_shmat(shp, shmaddr, shmflg);
+	return COND_SECURITY(shm_shmat(shp, shmaddr, shmflg), 
+ 			 0);
 }
 
 static inline int security_sem_alloc (struct sem_array *sma)
 {
-	return security_ops->sem_alloc_security (sma);
+	return COND_SECURITY(sem_alloc_security (sma), 
+ 			 0);
 }
 
 static inline void security_sem_free (struct sem_array *sma)
 {
-	security_ops->sem_free_security (sma);
+	COND_SECURITY(sem_free_security (sma),
+ 		  SE_NOP);
 }
 
 static inline int security_sem_associate (struct sem_array * sma, int semflg)
 {
-	return security_ops->sem_associate (sma, semflg);
+	return COND_SECURITY(sem_associate (sma, semflg), 
+ 			 0);
 }
 
 static inline int security_sem_semctl (struct sem_array * sma, int cmd)
 {
-	return security_ops->sem_semctl(sma, cmd);
+	return COND_SECURITY(sem_semctl(sma, cmd), 
+ 			 0);
 }
 
 static inline int security_sem_semop (struct sem_array * sma, 
 				      struct sembuf * sops, unsigned nsops, 
 				      int alter)
 {
-	return security_ops->sem_semop(sma, sops, nsops, alter);
+	return COND_SECURITY(sem_semop(sma, sops, nsops, alter), 
+ 			 0);
 }
 
 static inline void security_d_instantiate (struct dentry *dentry, struct inode *inode)
 {
-	security_ops->d_instantiate (dentry, inode);
+	COND_SECURITY(d_instantiate (dentry, inode),
+ 		  SE_NOP);
 }
 
 static inline int security_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
 {
-	return security_ops->getprocattr(p, name, value, size);
+	return COND_SECURITY(getprocattr(p, name, value, size), 
+ 			 -EINVAL);
 }
 
 static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
 {
-	return security_ops->setprocattr(p, name, value, size);
+	return COND_SECURITY(setprocattr(p, name, value, size), 
+ 			 -EINVAL);
 }
 
 static inline int security_netlink_send(struct sock *sk, struct sk_buff * skb)
 {
-	return security_ops->netlink_send(sk, skb);
+	return COND_SECURITY(netlink_send(sk, skb),
+ 			 cap_netlink_send (sk, skb));
 }
 
 static inline int security_netlink_recv(struct sk_buff * skb)
 {
-	return security_ops->netlink_recv(skb);
+	return COND_SECURITY(netlink_recv(skb),
+ 			 cap_netlink_recv (skb));
 }
 
 /* prototypes */
@@ -2521,20 +2654,23 @@
 					       struct socket * other, 
 					       struct sock * newsk)
 {
-	return security_ops->unix_stream_connect(sock, other, newsk);
+	return COND_SECURITY(unix_stream_connect(sock, other, newsk), 
+ 			 0);
 }
 
 
 static inline int security_unix_may_send(struct socket * sock, 
 					 struct socket * other)
 {
-	return security_ops->unix_may_send(sock, other);
+	return COND_SECURITY(unix_may_send(sock, other), 
+ 			 0);
 }
 
 static inline int security_socket_create (int family, int type,
 					  int protocol, int kern)
 {
-	return security_ops->socket_create(family, type, protocol, kern);
+	return COND_SECURITY(socket_create(family, type, protocol, kern), 
+ 			 0);
 }
 
 static inline void security_socket_post_create(struct socket * sock, 
@@ -2542,101 +2678,117 @@
 					       int type, 
 					       int protocol, int kern)
 {
-	security_ops->socket_post_create(sock, family, type,
-					 protocol, kern);
+	COND_SECURITY(socket_post_create(sock, family, type, protocol, kern), 
+ 		  SE_NOP);
 }
 
 static inline int security_socket_bind(struct socket * sock, 
 				       struct sockaddr * address, 
 				       int addrlen)
 {
-	return security_ops->socket_bind(sock, address, addrlen);
+	return COND_SECURITY(socket_bind(sock, address, addrlen), 
+ 			 0);
 }
 
 static inline int security_socket_connect(struct socket * sock, 
 					  struct sockaddr * address, 
 					  int addrlen)
 {
-	return security_ops->socket_connect(sock, address, addrlen);
+	return COND_SECURITY(socket_connect(sock, address, addrlen), 
+ 			 0);
 }
 
 static inline int security_socket_listen(struct socket * sock, int backlog)
 {
-	return security_ops->socket_listen(sock, backlog);
+	return COND_SECURITY(socket_listen(sock, backlog), 
+ 			 0);
 }
 
 static inline int security_socket_accept(struct socket * sock, 
 					 struct socket * newsock)
 {
-	return security_ops->socket_accept(sock, newsock);
+	return COND_SECURITY(socket_accept(sock, newsock), 
+ 			 0);
 }
 
 static inline void security_socket_post_accept(struct socket * sock, 
 					       struct socket * newsock)
 {
-	security_ops->socket_post_accept(sock, newsock);
+	COND_SECURITY(socket_post_accept(sock, newsock), 
+ 		  SE_NOP);
 }
 
 static inline int security_socket_sendmsg(struct socket * sock, 
 					  struct msghdr * msg, int size)
 {
-	return security_ops->socket_sendmsg(sock, msg, size);
+	return COND_SECURITY(socket_sendmsg(sock, msg, size), 
+ 			 0);
 }
 
 static inline int security_socket_recvmsg(struct socket * sock, 
 					  struct msghdr * msg, int size, 
 					  int flags)
 {
-	return security_ops->socket_recvmsg(sock, msg, size, flags);
+	return COND_SECURITY(socket_recvmsg(sock, msg, size, flags), 
+ 			 0);
 }
 
 static inline int security_socket_getsockname(struct socket * sock)
 {
-	return security_ops->socket_getsockname(sock);
+	return COND_SECURITY(socket_getsockname(sock), 
+ 			 0);
 }
 
 static inline int security_socket_getpeername(struct socket * sock)
 {
-	return security_ops->socket_getpeername(sock);
+	return COND_SECURITY(socket_getpeername(sock), 
+ 			 0);
 }
 
 static inline int security_socket_getsockopt(struct socket * sock, 
 					     int level, int optname)
 {
-	return security_ops->socket_getsockopt(sock, level, optname);
+	return COND_SECURITY(socket_getsockopt(sock, level, optname), 
+ 			 0);
 }
 
 static inline int security_socket_setsockopt(struct socket * sock, 
 					     int level, int optname)
 {
-	return security_ops->socket_setsockopt(sock, level, optname);
+	return COND_SECURITY(socket_setsockopt(sock, level, optname), 
+ 			 0);
 }
 
 static inline int security_socket_shutdown(struct socket * sock, int how)
 {
-	return security_ops->socket_shutdown(sock, how);
+	return COND_SECURITY(socket_shutdown(sock, how), 
+ 			 0);
 }
 
 static inline int security_sock_rcv_skb (struct sock * sk, 
 					 struct sk_buff * skb)
 {
-	return security_ops->socket_sock_rcv_skb (sk, skb);
+	return COND_SECURITY(socket_sock_rcv_skb (sk, skb), 
+ 			 0);
 }
 
 static inline int security_socket_getpeersec(struct socket *sock, char __user *optval,
 					     int __user *optlen, unsigned len)
 {
-	return security_ops->socket_getpeersec(sock, optval, optlen, len);
+	return COND_SECURITY(socket_getpeersec(sock, optval, optlen, len), 
+ 			 -ENOPROTOOPT);
 }
 
 static inline int security_sk_alloc(struct sock *sk, int family, int priority)
 {
-	return security_ops->sk_alloc_security(sk, family, priority);
+	return COND_SECURITY(sk_alloc_security(sk, family, priority), 
+ 			 0);
 }
 
 static inline void security_sk_free(struct sock *sk)
 {
-	return security_ops->sk_free_security(sk);
+	return COND_SECURITY(sk_free_security(sk), 
+ 		  	0);
 }
 #else	/* CONFIG_SECURITY_NETWORK */
 static inline int security_unix_stream_connect(struct socket * sock,
Index: linux-2.6.7/security/Makefile
===================================================================
--- linux-2.6.7.orig/security/Makefile	2004-08-05 14:58:22.439493894 +0200
+++ linux-2.6.7/security/Makefile	2004-08-05 15:00:29.598162747 +0200
@@ -4,14 +4,12 @@
 
 subdir-$(CONFIG_SECURITY_SELINUX)	+= selinux
 
-# if we don't select a security model, use the default capabilities
-ifneq ($(CONFIG_SECURITY),y)
+# We always need commoncap as it's default
 obj-y		+= commoncap.o
-endif
 
 # Object file lists
 obj-$(CONFIG_SECURITY)			+= security.o dummy.o
 # Must precede capability.o in order to stack properly.
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
-obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
-obj-$(CONFIG_SECURITY_ROOTPLUG)		+= commoncap.o root_plug.o
+obj-$(CONFIG_SECURITY_ROOTPLUG)		+= root_plug.o
+obj-$(CONFIG_SECURITY_CAPABILITIES)	+= capability.o
Index: linux-2.6.7/security/capability.c
===================================================================
--- linux-2.6.7.orig/security/capability.c	2004-08-05 14:58:22.440493797 +0200
+++ linux-2.6.7/security/capability.c	2004-08-05 15:26:01.959562780 +0200
@@ -24,29 +24,19 @@
 #include <linux/ptrace.h>
 #include <linux/moduleparam.h>
 
-static struct security_operations capability_ops = {
-	.ptrace =			cap_ptrace,
-	.capget =			cap_capget,
-	.capset_check =			cap_capset_check,
-	.capset_set =			cap_capset_set,
-	.capable =			cap_capable,
-	.netlink_send =			cap_netlink_send,
-	.netlink_recv =			cap_netlink_recv,
-
-	.bprm_apply_creds =		cap_bprm_apply_creds,
-	.bprm_set_security =		cap_bprm_set_security,
-	.bprm_secureexec =		cap_bprm_secureexec,
-
-	.inode_setxattr =		cap_inode_setxattr,
-	.inode_removexattr =		cap_inode_removexattr,
-
-	.task_post_setuid =		cap_task_post_setuid,
-	.task_reparent_to_init =	cap_task_reparent_to_init,
-
-	.syslog =                       cap_syslog,
-
-	.vm_enough_memory =             cap_vm_enough_memory,
-};
+/* Note: If the capability security module is loaded, we do NOT register
+ * the capability_security_ops but a second structure capability_ops
+ * that has the identical entries. The reasons:
+ * - we could stack on top of capability if it was stackable
+ * - a loaded capability module will prevent others to register, which
+ *   is the previous behaviour; if capabilities are used as default (not
+ *   because the module has been loaded), we allow the replacement.
+ */
+
+/* Struct from commoncaps */
+extern struct security_operations capability_security_ops;
+/* Struct to hold the copy */
+static struct security_operations capability_ops;
 
 #define MY_NAME __stringify(KBUILD_MODNAME)
 
@@ -59,6 +49,7 @@
 
 static int __init capability_init (void)
 {
+	memcpy(&capability_ops, &capability_security_ops, sizeof(capability_ops));
 	if (capability_disable) {
 		printk(KERN_INFO "Capabilities disabled at initialization\n");
 		return 0;
Index: linux-2.6.7/security/commoncap.c
===================================================================
--- linux-2.6.7.orig/security/commoncap.c	2004-08-05 14:58:22.440493797 +0200
+++ linux-2.6.7/security/commoncap.c	2004-08-05 14:58:54.161417677 +0200
@@ -367,6 +367,42 @@
 	return -ENOMEM;
 }
 
+#ifdef CONFIG_SECURITY
+struct security_operations capability_security_ops = {
+	.ptrace =			cap_ptrace,
+	.capget =			cap_capget,
+	.capset_check =			cap_capset_check,
+	.capset_set =			cap_capset_set,
+	.capable =			cap_capable,
+	.netlink_send =			cap_netlink_send,
+	.netlink_recv =			cap_netlink_recv,
+
+	.bprm_apply_creds =		cap_bprm_apply_creds,
+	.bprm_set_security =		cap_bprm_set_security,
+	.bprm_secureexec =		cap_bprm_secureexec,
+
+	.inode_setxattr =		cap_inode_setxattr,
+	.inode_removexattr =		cap_inode_removexattr,
+
+	.task_post_setuid =		cap_task_post_setuid,
+	.task_reparent_to_init =	cap_task_reparent_to_init,
+
+	.syslog =                       cap_syslog,
+
+	.vm_enough_memory =             cap_vm_enough_memory,
+};
+
+EXPORT_SYMBOL(capability_security_ops);
+/* Note: If the capability security module is loaded, we do NOT register
+ * the capability_security_ops but a second structure that has the
+ * identical entries. The reason is that this way,
+ * - we could stack on top of capability if it was stackable
+ * - a loaded capability module will prevent others to register, which
+ *   is the previous behaviour; if capabilities are used as default (not
+ *   because the module has been loaded), we allow the replacement.
+ */
+#endif
+
 EXPORT_SYMBOL(cap_capable);
 EXPORT_SYMBOL(cap_ptrace);
 EXPORT_SYMBOL(cap_capget);
Index: linux-2.6.7/security/dummy.c
===================================================================
--- linux-2.6.7.orig/security/dummy.c	2004-08-05 14:58:22.440493797 +0200
+++ linux-2.6.7/security/dummy.c	2004-08-05 14:58:54.162417580 +0200
@@ -873,9 +873,6 @@
 	return -EINVAL;
 }
 
-
-struct security_operations dummy_security_ops;
-
 #define set_to_dummy_if_null(ops, function)				\
 	do {								\
 		if (!ops->function) {					\
Index: linux-2.6.7/security/security.c
===================================================================
--- linux-2.6.7.orig/security/security.c	2004-08-05 14:58:22.440493797 +0200
+++ linux-2.6.7/security/security.c	2004-08-05 14:58:54.163417483 +0200
@@ -20,11 +20,42 @@
 
 #define SECURITY_SCAFFOLD_VERSION	"1.0.0"
 
+/* garloff@suse.de, 2004-05-21:
+ * lsm causes a performance problem, if compiled in, due to various
+ * non-inlined indirect function calls.
+ * This can be avoided by putting a branch in the inlined security
+ * stubs in include/linux/security.h, calling directly into the cap_
+ * functions from commoncap.
+ * This has some consequences:
+ * - If no security module is loaded, default will be the capability
+ *   security fns, not the dummy ones.
+ * - If a security module is loaded, it will override the defaults;
+ *   this module might be capability itself, overriding itself, 
+ *   only causing a slowdown. This means that capability should NOT 
+ *   be compiled into the kernel.
+ * - Another module can be loaded, and capability, being a module again,
+ *   might be stacked as secondary module.
+ * - Unfortunately, we can't get rid of dummy, as we don't want to
+ *   change the default behaviour if a security module is loaded and
+ *   some stubs are not implemented in which case these default to
+ *   dummy (which behaves differently to capability for some stubs). 
+ * - If no security module is loaded, we set security_ops to point
+ *   to capability_security_ops; it will not normally be used except for 
+ *   one situation: When a security module is unloaded; the value of 
+ *   security_enabled may still be evaluated to 1 when the security_ops 
+ *   is already changed. The behaviour is consistent here, as we do
+ *   change security_ops back to point to capability_security_ops.
+ * - commoncaps needs to be compiled in unconditionally.
+ */ 
+
 /* things that live in dummy.c */
-extern struct security_operations dummy_security_ops;
 extern void security_fixup_ops (struct security_operations *ops);
+/* default security ops */
+extern struct security_operations capability_security_ops;
 
 struct security_operations *security_ops;	/* Initialized to NULL */
+int security_enabled;				/* ditto */
+EXPORT_SYMBOL(security_enabled);
 
 static inline int verify (struct security_operations *ops)
 {
@@ -57,14 +88,16 @@
 {
 	printk (KERN_INFO "Security Scaffold v" SECURITY_SCAFFOLD_VERSION
 		" initialized\n");
-
-	if (verify (&dummy_security_ops)) {
+	
+	if (verify (&capability_security_ops)) {
 		printk (KERN_ERR "%s could not verify "
 			"dummy_security_ops structure.\n", __FUNCTION__);
 		return -EIO;
 	}
-
-	security_ops = &dummy_security_ops;
+	security_enabled = 0;
+	security_ops = &capability_security_ops;
+	
+	/* Init compiled-in security modules */
 	do_security_initcalls();
 
 	return 0;
@@ -90,13 +123,14 @@
 		return -EINVAL;
 	}
 
-	if (security_ops != &dummy_security_ops) {
+	if (security_ops != &capability_security_ops) {
 		printk (KERN_INFO "There is already a security "
 			"framework initialized, %s failed.\n", __FUNCTION__);
 		return -EINVAL;
 	}
 
 	security_ops = ops;
+	security_enabled = 1;
 
 	return 0;
 }
@@ -116,13 +150,14 @@
 {
 	if (ops != security_ops) {
 		printk (KERN_INFO "%s: trying to unregister "
-			"a security_opts structure that is not "
+			"a security_ops structure that is not "
 			"registered, failing.\n", __FUNCTION__);
 		return -EINVAL;
 	}
 
-	security_ops = &dummy_security_ops;
-
+	security_enabled = 0;
+	security_ops = &capability_security_ops;
+	
 	return 0;
 }
 

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10  8:57 [PATCH] [LSM] Rework LSM hooks Kurt Garloff
@ 2004-08-10 13:29 ` James Morris
  2004-08-10 20:23   ` Chris Wright
  2004-08-10 14:16 ` James Morris
  1 sibling, 1 reply; 26+ messages in thread
From: James Morris @ 2004-08-10 13:29 UTC (permalink / raw)
  To: Kurt Garloff; +Cc: Linux kernel list, Chris Wright, Stephen Smalley

On Tue, 10 Aug 2004, Kurt Garloff wrote:

> Hi,
> 
> while looking into LSM hooks and SElinux for SLES9, we came across
> a couple of issues and the whole thing ended up in a patch that I
> think should be applied upstream.
> 
> Some boundary conditions:
> * We don't want to use selinux by default: The complexity to set up a
>   secure system using it is currently quite complex
> * The impact of SElinux on performance on SMP is disastrous

This is a known issue and is being worked on.

> * SElinux needs to be compiled in
> * CONFIG_SECURITY should remain on -- it allows the flexibility to
>   use different LSMs
> * This however has the nasty side-effect of ending up with a system
>   that uses dummy rather than the Linux default capabilities; so your
>   boot up scripts need to take care of loading it. Making capability
>   static is no option either, of course, as you want to be able to
>   use a different primary LSM or load capability as secondary LSM.
> * Even with selinux=0 and capability loaded, the kernel takes a 
>   few percents in networking benchmarks (measured by HP on ia64); 
>   this is caused by the slowliness of indirect jumps on ia64.
> 
> The first patch patch does just change the selinux default; so you
> need to enable with selinux=1.

This issue has been through a couple of iterations and the current scheme
where if you have SELinux enabled, it is on by default, is aimed at being
more secure by default.  On some platforms, boot parameters are not
feasible.  To allow SELinux to be disable for these, the /selinux/disable
node was implemented, which allows SELinux to be unregistered during boot.  
I suggest you investigate using this; look at what Fedora does.


- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10  8:57 [PATCH] [LSM] Rework LSM hooks Kurt Garloff
  2004-08-10 13:29 ` James Morris
@ 2004-08-10 14:16 ` James Morris
  2004-08-10 15:02   ` Alan Cox
  2004-08-11 22:19   ` Kurt Garloff
  1 sibling, 2 replies; 26+ messages in thread
From: James Morris @ 2004-08-10 14:16 UTC (permalink / raw)
  To: Kurt Garloff; +Cc: Linux kernel list, Chris Wright, Stephen Smalley, Greg KH

On Tue, 10 Aug 2004, Kurt Garloff wrote:

> * Even with selinux=0 and capability loaded, the kernel takes a 
>   few percents in networking benchmarks (measured by HP on ia64); 
>   this is caused by the slowliness of indirect jumps on ia64.

Is this just an ia64 issue?  If so, then perhaps we should look at only
penalising ia64?  Otherwise, loading an LSM module is going to cause
expensive false unlikely() on _every_ LSM hook.


- James
-- 
James Morris
<jmorris@redhat.com>





^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 14:16 ` James Morris
@ 2004-08-10 15:02   ` Alan Cox
  2004-08-10 19:22     ` James Morris
  2004-08-11 22:19   ` Kurt Garloff
  1 sibling, 1 reply; 26+ messages in thread
From: Alan Cox @ 2004-08-10 15:02 UTC (permalink / raw)
  To: James Morris
  Cc: Kurt Garloff, Linux Kernel Mailing List, Chris Wright,
	Stephen Smalley, Greg KH

On Maw, 2004-08-10 at 15:16, James Morris wrote:
> On Tue, 10 Aug 2004, Kurt Garloff wrote:
> 
> > * Even with selinux=0 and capability loaded, the kernel takes a 
> >   few percents in networking benchmarks (measured by HP on ia64); 
> >   this is caused by the slowliness of indirect jumps on ia64.
> 
> Is this just an ia64 issue?  If so, then perhaps we should look at only
> penalising ia64?  Otherwise, loading an LSM module is going to cause
> expensive false unlikely() on _every_ LSM hook.

I see this on x86-32 to an extent. Its quite visible with gigabit as
you'd expect. ia64 ought to be less affected providing the compiler is
doing the right things with the unconditional jumps.


Alan


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 15:02   ` Alan Cox
@ 2004-08-10 19:22     ` James Morris
  2004-08-10 20:00       ` Chris Wright
  0 siblings, 1 reply; 26+ messages in thread
From: James Morris @ 2004-08-10 19:22 UTC (permalink / raw)
  To: Alan Cox
  Cc: Kurt Garloff, Linux Kernel Mailing List, Chris Wright,
	Stephen Smalley, Greg KH

On Tue, 10 Aug 2004, Alan Cox wrote:

> On Maw, 2004-08-10 at 15:16, James Morris wrote:
> > On Tue, 10 Aug 2004, Kurt Garloff wrote:
> > 
> > > * Even with selinux=0 and capability loaded, the kernel takes a 
> > >   few percents in networking benchmarks (measured by HP on ia64); 
> > >   this is caused by the slowliness of indirect jumps on ia64.
> > 
> > Is this just an ia64 issue?  If so, then perhaps we should look at only
> > penalising ia64?  Otherwise, loading an LSM module is going to cause
> > expensive false unlikely() on _every_ LSM hook.
> 
> I see this on x86-32 to an extent. Its quite visible with gigabit as
> you'd expect. ia64 ought to be less affected providing the compiler is
> doing the right things with the unconditional jumps.

I did some benchmarking (full results below), and I'm not seeing anything
significant on a P4 Xeon.

For a vanilla kernel (no LSM):

  Looback TCP bandwidth = 305.9 MB/s over ten lmbench runs.
  Apachebench loobpack, 100k requests for a 50k file: 2045.70 reqs/s

For a kernel with LSM + networking hooks + capability module:

  Looback TCP bandwidth = 305.8 MB/s
  pachebench loobpack, 100k requests for a 50k file: 2044.29 req/s

The overhead of LSM on networking performance in these tests is in the 
noise.  The ab test was even done with httpd logging enabled on ext3.


- James
-- 
James Morris
<jmorris@redhat.com>


Full benchmark results:

--------------------------------------------------------------------------------
'Vanilla' kernel, no Netfilter, No LSM
--------------------------------------------------------------------------------

1) Apachebench loopback

# ab -n 100000 -c 2 http://localhost/index.html
This is ApacheBench, Version 2.0.40-dev <$Revision: 1.141 $> apache-2.0
Copyright (c) 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Copyright (c) 1998-2002 The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)
Completed 10000 requests
Completed 20000 requests
Completed 30000 requests
Completed 40000 requests
Completed 50000 requests
Completed 60000 requests
Completed 70000 requests
Completed 80000 requests
Completed 90000 requests
Finished 100000 requests


Server Software:        Apache/2.0.49
Server Hostname:        localhost
Server Port:            80

Document Path:          /index.html
Document Length:        51200 bytes

Concurrency Level:      2
Time taken for tests:   48.882919 seconds
Complete requests:      100000
Failed requests:        0
Write errors:           0
Total transferred:      851732704 bytes
HTML transferred:       825032704 bytes
Requests per second:    2045.70 [#/sec] (mean)
Time per request:       0.978 [ms] (mean)
Time per request:       0.489 [ms] (mean, across all concurrent requests)
Transfer rate:          17015.55 [Kbytes/sec] received

Connection Times (ms)
               min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       1
Processing:     0    0   1.0      0     159
Waiting:        0    0   0.8      0     158
Total:          0    0   1.0      0     159
              
Percentage of the requests served within a certain time (ms)
50%      0
66%      0
75%      0
80%      0
90%      0
95%      1
98%      1
99%      1
100%    159 (longest request)
                               

2) lmbench loopback TCP peformance:

bw_tcp

0.065536 303.62 MB/sec
0.065536 306.19 MB/sec
0.065536 309.81 MB/sec
0.065536 309.18 MB/sec
0.065536 303.98 MB/sec
0.065536 305.58 MB/sec
0.065536 305.86 MB/sec
0.065536 301.23 MB/sec
0.065536 306.08 MB/sec
0.065536 307.63 MB/sec
av. = 305.9 MB/s


--------------------------------------------------------------------------------
'Test' kernel, no Netfilter, LSM with networking hooks + static capability mod
--------------------------------------------------------------------------------

1) Apachebench loobpack:

ab -n 100000 -c 2 http://localhost/index.html
This is ApacheBench, Version 2.0.40-dev <$Revision: 1.141 $> apache-2.0
Copyright (c) 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Copyright (c) 1998-2002 The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)
Completed 10000 requests
Completed 20000 requests
Completed 30000 requests
Completed 40000 requests
Completed 50000 requests
Completed 60000 requests
Completed 70000 requests
Completed 80000 requests
Completed 90000 requests
Finished 100000 requests


Server Software:        Apache/2.0.49
Server Hostname:        localhost
Server Port:            80

Document Path:          /index.html
Document Length:        51200 bytes

Concurrency Level:      2
Time taken for tests:   48.916767 seconds
Complete requests:      100000
Failed requests:        0
Write errors:           0
Total transferred:      851732704 bytes
HTML transferred:       825032704 bytes
Requests per second:    2044.29 [#/sec] (mean)
Time per request:       0.978 [ms] (mean)
Time per request:       0.489 [ms] (mean, across all concurrent requests)
Transfer rate:          17003.78 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       1
Processing:     0    0   1.3      0     260
Waiting:        0    0   1.2      0     260
Total:          0    0   1.3      0     260
              
Percentage of the requests served within a certain time (ms)
50%      0
66%      0
75%      0
80%      0
90%      0
95%      1
98%      1
99%      1
100%    260 (longest request)



2) lmbench loopback TCP peformance:

bw_tcp

0.065536 306.08 MB/sec
0.065536 304.87 MB/sec
0.065536 302.42 MB/sec
0.065536 306.08 MB/sec
0.065536 306.24 MB/sec
0.065536 305.61 MB/sec
0.065536 307.04 MB/sec
0.065536 308.74 MB/sec
0.065536 305.97 MB/sec
0.065536 304.57 MB/sec
av. = 305.8 MB/s













^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 19:22     ` James Morris
@ 2004-08-10 20:00       ` Chris Wright
  2004-08-10 20:07         ` James Morris
  0 siblings, 1 reply; 26+ messages in thread
From: Chris Wright @ 2004-08-10 20:00 UTC (permalink / raw)
  To: James Morris
  Cc: Alan Cox, Kurt Garloff, Linux Kernel Mailing List, Chris Wright,
	Stephen Smalley, Greg KH

* James Morris (jmorris@redhat.com) wrote:
> On Tue, 10 Aug 2004, Alan Cox wrote:
> 
> > On Maw, 2004-08-10 at 15:16, James Morris wrote:
> > > On Tue, 10 Aug 2004, Kurt Garloff wrote:
> > > 
> > > > * Even with selinux=0 and capability loaded, the kernel takes a 
> > > >   few percents in networking benchmarks (measured by HP on ia64); 
> > > >   this is caused by the slowliness of indirect jumps on ia64.
> > > 
> > > Is this just an ia64 issue?  If so, then perhaps we should look at only
> > > penalising ia64?  Otherwise, loading an LSM module is going to cause
> > > expensive false unlikely() on _every_ LSM hook.
> > 
> > I see this on x86-32 to an extent. Its quite visible with gigabit as
> > you'd expect. ia64 ought to be less affected providing the compiler is
> > doing the right things with the unconditional jumps.
> 
> I did some benchmarking (full results below), and I'm not seeing anything
> significant on a P4 Xeon.

Is this new (i.e. you just did this)?  It's basically the same result we
had from a few years ago.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:00       ` Chris Wright
@ 2004-08-10 20:07         ` James Morris
  2004-08-10 20:12           ` Chris Wright
  0 siblings, 1 reply; 26+ messages in thread
From: James Morris @ 2004-08-10 20:07 UTC (permalink / raw)
  To: Chris Wright
  Cc: Alan Cox, Kurt Garloff, Linux Kernel Mailing List,
	Stephen Smalley, Greg KH

On Tue, 10 Aug 2004, Chris Wright wrote:

> > I did some benchmarking (full results below), and I'm not seeing anything
> > significant on a P4 Xeon.
> 
> Is this new (i.e. you just did this)?  It's basically the same result we
> had from a few years ago.

Yes, did it today.


- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:07         ` James Morris
@ 2004-08-10 20:12           ` Chris Wright
  2004-08-10 20:31             ` James Morris
  2004-08-11 22:22             ` Kurt Garloff
  0 siblings, 2 replies; 26+ messages in thread
From: Chris Wright @ 2004-08-10 20:12 UTC (permalink / raw)
  To: James Morris
  Cc: Chris Wright, Alan Cox, Kurt Garloff, Linux Kernel Mailing List,
	Stephen Smalley, Greg KH

* James Morris (jmorris@redhat.com) wrote:
> On Tue, 10 Aug 2004, Chris Wright wrote:
> > Is this new (i.e. you just did this)?  It's basically the same result we
> > had from a few years ago.
> 
> Yes, did it today.

Thanks, James.  Since these are the only concrete numbers and they are
in the noise, I see no compelling reason to change to unlikely().

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 13:29 ` James Morris
@ 2004-08-10 20:23   ` Chris Wright
  2004-08-10 20:27     ` James Morris
  0 siblings, 1 reply; 26+ messages in thread
From: Chris Wright @ 2004-08-10 20:23 UTC (permalink / raw)
  To: James Morris
  Cc: Kurt Garloff, Linux kernel list, Chris Wright, Stephen Smalley

* James Morris (jmorris@redhat.com) wrote:
> On Tue, 10 Aug 2004, Kurt Garloff wrote:
> > The first patch patch does just change the selinux default; so you
> > need to enable with selinux=1.
> 
> This issue has been through a couple of iterations and the current scheme
> where if you have SELinux enabled, it is on by default, is aimed at being
> more secure by default.  On some platforms, boot parameters are not
> feasible.  To allow SELinux to be disable for these, the /selinux/disable
> node was implemented, which allows SELinux to be unregistered during boot.  
> I suggest you investigate using this; look at what Fedora does.

Could make selinux_enabled value configurable.  I don't really like the
extra configuration, but if it's more vendor neutral to have config
not only control if you can have bootparam, but also default value,
then perhaps it'd be useful.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:23   ` Chris Wright
@ 2004-08-10 20:27     ` James Morris
  2004-08-10 20:43       ` Chris Wright
  0 siblings, 1 reply; 26+ messages in thread
From: James Morris @ 2004-08-10 20:27 UTC (permalink / raw)
  To: Chris Wright; +Cc: Kurt Garloff, Linux kernel list, Stephen Smalley

On Tue, 10 Aug 2004, Chris Wright wrote:

> * James Morris (jmorris@redhat.com) wrote:
> > On Tue, 10 Aug 2004, Kurt Garloff wrote:
> > > The first patch patch does just change the selinux default; so you
> > > need to enable with selinux=1.
> > 
> > This issue has been through a couple of iterations and the current scheme
> > where if you have SELinux enabled, it is on by default, is aimed at being
> > more secure by default.  On some platforms, boot parameters are not
> > feasible.  To allow SELinux to be disable for these, the /selinux/disable
> > node was implemented, which allows SELinux to be unregistered during boot.  
> > I suggest you investigate using this; look at what Fedora does.
> 
> Could make selinux_enabled value configurable.  I don't really like the
> extra configuration, but if it's more vendor neutral to have config
> not only control if you can have bootparam, but also default value,
> then perhaps it'd be useful.

Config option sounds fine to me.

- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:12           ` Chris Wright
@ 2004-08-10 20:31             ` James Morris
  2004-08-11  8:47               ` David Mosberger
  2004-08-11 22:22             ` Kurt Garloff
  1 sibling, 1 reply; 26+ messages in thread
From: James Morris @ 2004-08-10 20:31 UTC (permalink / raw)
  To: Chris Wright
  Cc: Alan Cox, Kurt Garloff, Linux Kernel Mailing List,
	Stephen Smalley, Greg KH

On Tue, 10 Aug 2004, Chris Wright wrote:

> Thanks, James.  Since these are the only concrete numbers and they are
> in the noise, I see no compelling reason to change to unlikely().

There may be some way to make it ia64 specific.  Is it a cpu issue, or 
compiler?


- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:27     ` James Morris
@ 2004-08-10 20:43       ` Chris Wright
  2004-08-11  1:55         ` James Morris
  0 siblings, 1 reply; 26+ messages in thread
From: Chris Wright @ 2004-08-10 20:43 UTC (permalink / raw)
  To: James Morris
  Cc: Chris Wright, Kurt Garloff, Linux kernel list, Stephen Smalley

* James Morris (jmorris@redhat.com) wrote:
> On Tue, 10 Aug 2004, Chris Wright wrote:
> 
> > * James Morris (jmorris@redhat.com) wrote:
> > > On Tue, 10 Aug 2004, Kurt Garloff wrote:
> > > > The first patch patch does just change the selinux default; so you
> > > > need to enable with selinux=1.
> > > 
> > > This issue has been through a couple of iterations and the current scheme
> > > where if you have SELinux enabled, it is on by default, is aimed at being
> > > more secure by default.  On some platforms, boot parameters are not
> > > feasible.  To allow SELinux to be disable for these, the /selinux/disable
> > > node was implemented, which allows SELinux to be unregistered during boot.  
> > > I suggest you investigate using this; look at what Fedora does.
> > 
> > Could make selinux_enabled value configurable.  I don't really like the
> > extra configuration, but if it's more vendor neutral to have config
> > not only control if you can have bootparam, but also default value,
> > then perhaps it'd be useful.
> 
> Config option sounds fine to me.

I'll push this up, unless there's an objection.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== security/selinux/Kconfig 1.6 vs edited =====
--- 1.6/security/selinux/Kconfig	2004-06-01 02:27:56 -07:00
+++ edited/security/selinux/Kconfig	2004-08-10 13:39:43 -07:00
@@ -24,6 +24,21 @@
 
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_SELINUX_BOOTPARAM_VALUE
+	int "NSA SELinux boot parameter default value"
+	depends on SECURITY_SELINUX_BOOTPARAM
+	range 0 1
+	default 1
+	help
+	  This option sets the default value for the kernel parameter
+	  'selinux', which allows SELinux to be disabled at boot.  If this
+	  option is set to 0 (zero), the SELinux kernel parameter will
+	  default to 0, disabling SELinux at bootup.  If this option is
+	  set to 1 (one), the SELinux kernel paramater will default to 1,
+	  enabling SELinux at bootup.
+
+	  If you are unsure how to answer this question, answer 1.
+
 config SECURITY_SELINUX_DISABLE
 	bool "NSA SELinux runtime disable"
 	depends on SECURITY_SELINUX
===== security/selinux/hooks.c 1.53 vs edited =====
--- 1.53/security/selinux/hooks.c	2004-07-28 21:58:32 -07:00
+++ edited/security/selinux/hooks.c	2004-08-10 13:44:00 -07:00
@@ -87,7 +87,7 @@
 #endif
 
 #ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
-int selinux_enabled = 1;
+int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;
 
 static int __init selinux_enabled_setup(char *str)
 {

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:43       ` Chris Wright
@ 2004-08-11  1:55         ` James Morris
  0 siblings, 0 replies; 26+ messages in thread
From: James Morris @ 2004-08-11  1:55 UTC (permalink / raw)
  To: Chris Wright; +Cc: Kurt Garloff, Linux kernel list, Stephen Smalley

On Tue, 10 Aug 2004, Chris Wright wrote:

> I'll push this up, unless there's an objection.

Looks fine to me.


- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:31             ` James Morris
@ 2004-08-11  8:47               ` David Mosberger
  2004-08-11 15:25                 ` Chris Wright
  0 siblings, 1 reply; 26+ messages in thread
From: David Mosberger @ 2004-08-11  8:47 UTC (permalink / raw)
  To: James Morris
  Cc: Chris Wright, Alan Cox, Kurt Garloff, Linux Kernel Mailing List,
	Stephen Smalley, Greg KH

>>>>> On Tue, 10 Aug 2004 16:31:12 -0400 (EDT), James Morris <jmorris@redhat.com> said:

  James> On Tue, 10 Aug 2004, Chris Wright wrote:
  >> Thanks, James.  Since these are the only concrete numbers and
  >> they are in the noise, I see no compelling reason to change to
  >> unlikely().

  James> There may be some way to make it ia64 specific.  Is it a cpu
  James> issue, or compiler?

I'm pretty sure the "unlikely()" part could be dropped with little/no
downside.  The part that's relatively expensive (10 cycles when
mispredicted) is the indirect call.  GCC doesn't handle this well on
ia64 and as a result, most indirect calls are mispredicted.

An alternative solution might be to have a call_likely() macro, where
you could predict the most likely target of an indirect call.  Perhaps
that could help other platforms as well.

	--david

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-11  8:47               ` David Mosberger
@ 2004-08-11 15:25                 ` Chris Wright
  2004-08-11 18:12                   ` David Mosberger
  0 siblings, 1 reply; 26+ messages in thread
From: Chris Wright @ 2004-08-11 15:25 UTC (permalink / raw)
  To: davidm
  Cc: James Morris, Chris Wright, Alan Cox, Kurt Garloff,
	Linux Kernel Mailing List, Stephen Smalley, Greg KH

* David Mosberger (davidm@napali.hpl.hp.com) wrote:
> >>>>> On Tue, 10 Aug 2004 16:31:12 -0400 (EDT), James Morris <jmorris@redhat.com> said:
> 
>   James> On Tue, 10 Aug 2004, Chris Wright wrote:
>   >> Thanks, James.  Since these are the only concrete numbers and
>   >> they are in the noise, I see no compelling reason to change to
>   >> unlikely().
> 
>   James> There may be some way to make it ia64 specific.  Is it a cpu
>   James> issue, or compiler?
> 
> I'm pretty sure the "unlikely()" part could be dropped with little/no
> downside.  The part that's relatively expensive (10 cycles when
> mispredicted) is the indirect call.  GCC doesn't handle this well on
> ia64 and as a result, most indirect calls are mispredicted.
> 
> An alternative solution might be to have a call_likely() macro, where
> you could predict the most likely target of an indirect call.  Perhaps
> that could help other platforms as well.

Hmm, the pointers are generally quite static, set once near boot time
typically, and that's it.  Seems like a plausible win.  Do you have an
example of what call_likely() would look like?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-11 15:25                 ` Chris Wright
@ 2004-08-11 18:12                   ` David Mosberger
  2004-08-11 18:16                     ` Chris Wright
  0 siblings, 1 reply; 26+ messages in thread
From: David Mosberger @ 2004-08-11 18:12 UTC (permalink / raw)
  To: Chris Wright
  Cc: davidm, James Morris, Alan Cox, Kurt Garloff,
	Linux Kernel Mailing List, Stephen Smalley, Greg KH

>>>>> On Wed, 11 Aug 2004 08:25:10 -0700, Chris Wright <chrisw@osdl.org> said:

  >> An alternative solution might be to have a call_likely() macro,
  >> where you could predict the most likely target of an indirect
  >> call.  Perhaps that could help other platforms as well.

  Chris> Hmm, the pointers are generally quite static, set once near
  Chris> boot time typically, and that's it.  Seems like a plausible
  Chris> win.  Do you have an example of what call_likely() would look
  Chris> like?

The macro I had in mind works only for static (compile-time)
predictions, I'm afraid.

	--david

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-11 18:12                   ` David Mosberger
@ 2004-08-11 18:16                     ` Chris Wright
  0 siblings, 0 replies; 26+ messages in thread
From: Chris Wright @ 2004-08-11 18:16 UTC (permalink / raw)
  To: davidm
  Cc: Chris Wright, James Morris, Alan Cox, Kurt Garloff,
	Linux Kernel Mailing List, Stephen Smalley, Greg KH

* David Mosberger (davidm@napali.hpl.hp.com) wrote:
> The macro I had in mind works only for static (compile-time)
> predictions, I'm afraid.

Ah, shoot.  Well, I don't like the penalty from the unlikely() approach,
so we should keep it as-is unless there's another clever solution lurking.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 14:16 ` James Morris
  2004-08-10 15:02   ` Alan Cox
@ 2004-08-11 22:19   ` Kurt Garloff
  2004-08-12  1:20     ` James Morris
  1 sibling, 1 reply; 26+ messages in thread
From: Kurt Garloff @ 2004-08-11 22:19 UTC (permalink / raw)
  To: James Morris; +Cc: Linux kernel list

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

On Tue, Aug 10, 2004 at 10:16:29AM -0400, James Morris wrote:
> Is this just an ia64 issue?  If so, then perhaps we should look at only
> penalising ia64?  Otherwise, loading an LSM module is going to cause
> expensive false unlikely() on _every_ LSM hook.

You should worry about the fast path.
That's no LSM being loaded and just using the default capabilities.
Which is what most users usse as of this time.
If you do call into any serious LSM, you'll spend much more CPU cycles
anyway ...

Regards,
-- 
Kurt Garloff                   <kurt@garloff.de>             [Koeln, DE]
Physics:Plasma modeling <garloff@plasimo.phys.tue.nl> [TU Eindhoven, NL]
Linux: SUSE Labs (Head)        <garloff@suse.de>    [SUSE Nuernberg, DE]

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-10 20:12           ` Chris Wright
  2004-08-10 20:31             ` James Morris
@ 2004-08-11 22:22             ` Kurt Garloff
  2004-08-12  1:23               ` James Morris
  2004-08-16 14:19               ` Takashi Iwai
  1 sibling, 2 replies; 26+ messages in thread
From: Kurt Garloff @ 2004-08-11 22:22 UTC (permalink / raw)
  To: Chris Wright; +Cc: Linux kernel list

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

On Tue, Aug 10, 2004 at 01:12:17PM -0700, Chris Wright wrote:
> * James Morris (jmorris@redhat.com) wrote:
> > On Tue, 10 Aug 2004, Chris Wright wrote:
> > > Is this new (i.e. you just did this)?  It's basically the same result we
> > > had from a few years ago.
> > 
> > Yes, did it today.
> 
> Thanks, James.  Since these are the only concrete numbers and they are
> in the noise, I see no compelling reason to change to unlikely().

Well, you may want to drop the unlikely if you dislike it.
The rest of the path is still a win IMVHO.

Unfortunately, it has not been discussed here yet.

Reards,
-- 
Kurt Garloff                   <kurt@garloff.de>             [Koeln, DE]
Physics:Plasma modeling <garloff@plasimo.phys.tue.nl> [TU Eindhoven, NL]
Linux: SUSE Labs (Head)        <garloff@suse.de>    [SUSE Nuernberg, DE]

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-11 22:19   ` Kurt Garloff
@ 2004-08-12  1:20     ` James Morris
  2004-08-12  2:03       ` Kurt Garloff
  0 siblings, 1 reply; 26+ messages in thread
From: James Morris @ 2004-08-12  1:20 UTC (permalink / raw)
  To: Kurt Garloff; +Cc: Linux kernel list

On Thu, 12 Aug 2004, Kurt Garloff wrote:

> On Tue, Aug 10, 2004 at 10:16:29AM -0400, James Morris wrote:
> > Is this just an ia64 issue?  If so, then perhaps we should look at only
> > penalising ia64?  Otherwise, loading an LSM module is going to cause
> > expensive false unlikely() on _every_ LSM hook.
> 
> You should worry about the fast path.
> That's no LSM being loaded and just using the default capabilities.
> Which is what most users usse as of this time.

I'm not sure we can expect this to be true in the future.

> If you do call into any serious LSM, you'll spend much more CPU cycles
> anyway ...

Possibly, but keep in mind that your patch effectively adds 135 false
unlikely() calls throughout the kernel when an LSM is loaded.  Can you
provide figures for, say, the overhead of your patch (if any) with the BSD
securelevels LSM loaded?

Also, we still have the option of making COND_SECURITY ia64-specific.


- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-11 22:22             ` Kurt Garloff
@ 2004-08-12  1:23               ` James Morris
  2004-08-12  3:58                 ` Greg KH
  2004-08-12 18:17                 ` Chris Wright
  2004-08-16 14:19               ` Takashi Iwai
  1 sibling, 2 replies; 26+ messages in thread
From: James Morris @ 2004-08-12  1:23 UTC (permalink / raw)
  To: Kurt Garloff; +Cc: Chris Wright, Linux kernel list, Stephen Smalley, Greg KH

On Thu, 12 Aug 2004, Kurt Garloff wrote:

> The rest of the path is still a win IMVHO.
> 
> Unfortunately, it has not been discussed here yet.

Defaulting to the capability module is a separate discussion.  I
personally don't have a strong opinion on this issue.

Chris, Stephen, Greg? :-)



- James
-- 
James Morris
<jmorris@redhat.com>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-12  1:20     ` James Morris
@ 2004-08-12  2:03       ` Kurt Garloff
  0 siblings, 0 replies; 26+ messages in thread
From: Kurt Garloff @ 2004-08-12  2:03 UTC (permalink / raw)
  To: James Morris; +Cc: Linux kernel list

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

Hi James,

On Wed, Aug 11, 2004 at 09:20:22PM -0400, James Morris wrote:
> Also, we still have the option of making COND_SECURITY ia64-specific.

We could do that. The patch sets security_ops to
capabilities_security_ops if no LSM is loaded, so it would be OK to
call into it unconditionally on archs that have a higher penalty for
a branch than for an indirect call.

You could just redefine the macro, depending on the arch, indeed.

We could also drop the unlikely. For the hot paths, the branch
prediction of the CPU can do its job. So, I'm not religious about
it; in practice it should make little difference either way. 

My patch was aiming for a zerocost possibility to turn CONFIG_SECURITY
on. With the unlikely(), I should have gotten as close as one ~1 or 2 
CPU cycles (compare plus correctly predicted branch). That's why I
put the unlikely.

Regards,
-- 
Kurt Garloff                   <kurt@garloff.de>             [Koeln, DE]
Physics:Plasma modeling <garloff@plasimo.phys.tue.nl> [TU Eindhoven, NL]
Linux: SUSE Labs (Head)        <garloff@suse.de>    [SUSE Nuernberg, DE]

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-12  1:23               ` James Morris
@ 2004-08-12  3:58                 ` Greg KH
  2004-08-12 18:15                   ` Chris Wright
  2004-08-12 18:17                 ` Chris Wright
  1 sibling, 1 reply; 26+ messages in thread
From: Greg KH @ 2004-08-12  3:58 UTC (permalink / raw)
  To: James Morris
  Cc: Kurt Garloff, Chris Wright, Linux kernel list, Stephen Smalley

On Wed, Aug 11, 2004 at 09:23:19PM -0400, James Morris wrote:
> On Thu, 12 Aug 2004, Kurt Garloff wrote:
> 
> > The rest of the path is still a win IMVHO.
> > 
> > Unfortunately, it has not been discussed here yet.
> 
> Defaulting to the capability module is a separate discussion.  I
> personally don't have a strong opinion on this issue.
> 
> Chris, Stephen, Greg? :-)

I don't really care.  But I think almost every LSM module so far really
wants the capabilities module at the same time, right?  Maybe we should
make it easier to do that than what is necessary today.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-12  3:58                 ` Greg KH
@ 2004-08-12 18:15                   ` Chris Wright
  0 siblings, 0 replies; 26+ messages in thread
From: Chris Wright @ 2004-08-12 18:15 UTC (permalink / raw)
  To: Greg KH
  Cc: James Morris, Kurt Garloff, Chris Wright, Linux kernel list,
	Stephen Smalley

* Greg KH (greg@kroah.com) wrote:
> On Wed, Aug 11, 2004 at 09:23:19PM -0400, James Morris wrote:
> > On Thu, 12 Aug 2004, Kurt Garloff wrote:
> > 
> > > The rest of the path is still a win IMVHO.
> > > 
> > > Unfortunately, it has not been discussed here yet.
> > 
> > Defaulting to the capability module is a separate discussion.  I
> > personally don't have a strong opinion on this issue.
> > 
> > Chris, Stephen, Greg? :-)
> 
> I don't really care.  But I think almost every LSM module so far really
> wants the capabilities module at the same time, right?  Maybe we should
> make it easier to do that than what is necessary today.

Yes, we should.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-12  1:23               ` James Morris
  2004-08-12  3:58                 ` Greg KH
@ 2004-08-12 18:17                 ` Chris Wright
  1 sibling, 0 replies; 26+ messages in thread
From: Chris Wright @ 2004-08-12 18:17 UTC (permalink / raw)
  To: James Morris
  Cc: Kurt Garloff, Chris Wright, Linux kernel list, Stephen Smalley, Greg KH

* James Morris (jmorris@redhat.com) wrote:
> On Thu, 12 Aug 2004, Kurt Garloff wrote:
> 
> > The rest of the path is still a win IMVHO.
> > 
> > Unfortunately, it has not been discussed here yet.
> 
> Defaulting to the capability module is a separate discussion.  I
> personally don't have a strong opinion on this issue.

I've considering doing something like this.  Any chance you could
separate that bit of the patch out to work with?

thanks
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] [LSM] Rework LSM hooks
  2004-08-11 22:22             ` Kurt Garloff
  2004-08-12  1:23               ` James Morris
@ 2004-08-16 14:19               ` Takashi Iwai
  1 sibling, 0 replies; 26+ messages in thread
From: Takashi Iwai @ 2004-08-16 14:19 UTC (permalink / raw)
  To: Kurt Garloff; +Cc: Chris Wright, Linux kernel list

At Thu, 12 Aug 2004 00:22:13 +0200,
Kurt Garloff wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> On Tue, Aug 10, 2004 at 01:12:17PM -0700, Chris Wright wrote:
> > * James Morris (jmorris@redhat.com) wrote:
> > > On Tue, 10 Aug 2004, Chris Wright wrote:
> > > > Is this new (i.e. you just did this)?  It's basically the same result we
> > > > had from a few years ago.
> > > 
> > > Yes, did it today.
> > 
> > Thanks, James.  Since these are the only concrete numbers and they are
> > in the noise, I see no compelling reason to change to unlikely().
> 
> Well, you may want to drop the unlikely if you dislike it.

It can be dependent on CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE?
(E.g. use unlikely() when it's 0)


Takashi

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2004-08-16 14:22 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-08-10  8:57 [PATCH] [LSM] Rework LSM hooks Kurt Garloff
2004-08-10 13:29 ` James Morris
2004-08-10 20:23   ` Chris Wright
2004-08-10 20:27     ` James Morris
2004-08-10 20:43       ` Chris Wright
2004-08-11  1:55         ` James Morris
2004-08-10 14:16 ` James Morris
2004-08-10 15:02   ` Alan Cox
2004-08-10 19:22     ` James Morris
2004-08-10 20:00       ` Chris Wright
2004-08-10 20:07         ` James Morris
2004-08-10 20:12           ` Chris Wright
2004-08-10 20:31             ` James Morris
2004-08-11  8:47               ` David Mosberger
2004-08-11 15:25                 ` Chris Wright
2004-08-11 18:12                   ` David Mosberger
2004-08-11 18:16                     ` Chris Wright
2004-08-11 22:22             ` Kurt Garloff
2004-08-12  1:23               ` James Morris
2004-08-12  3:58                 ` Greg KH
2004-08-12 18:15                   ` Chris Wright
2004-08-12 18:17                 ` Chris Wright
2004-08-16 14:19               ` Takashi Iwai
2004-08-11 22:19   ` Kurt Garloff
2004-08-12  1:20     ` James Morris
2004-08-12  2:03       ` Kurt Garloff

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.