From f06febc96ba8e0af80bcc3eaec0a109e88275fac Mon Sep 17 00:00:00 2001 From: Frank Mayhar Date: Fri, 12 Sep 2008 09:54:39 -0700 Subject: timers: fix itimer/many thread hang Overview This patch reworks the handling of POSIX CPU timers, including the ITIMER_PROF, ITIMER_VIRT timers and rlimit handling. It was put together with the help of Roland McGrath, the owner and original writer of this code. The problem we ran into, and the reason for this rework, has to do with using a profiling timer in a process with a large number of threads. It appears that the performance of the old implementation of run_posix_cpu_timers() was at least O(n*3) (where "n" is the number of threads in a process) or worse. Everything is fine with an increasing number of threads until the time taken for that routine to run becomes the same as or greater than the tick time, at which point things degrade rather quickly. This patch fixes bug 9906, "Weird hang with NPTL and SIGPROF." Code Changes This rework corrects the implementation of run_posix_cpu_timers() to make it run in constant time for a particular machine. (Performance may vary between one machine and another depending upon whether the kernel is built as single- or multiprocessor and, in the latter case, depending upon the number of running processors.) To do this, at each tick we now update fields in signal_struct as well as task_struct. The run_posix_cpu_timers() function uses those fields to make its decisions. We define a new structure, "task_cputime," to contain user, system and scheduler times and use these in appropriate places: struct task_cputime { cputime_t utime; cputime_t stime; unsigned long long sum_exec_runtime; }; This is included in the structure "thread_group_cputime," which is a new substructure of signal_struct and which varies for uniprocessor versus multiprocessor kernels. For uniprocessor kernels, it uses "task_cputime" as a simple substructure, while for multiprocessor kernels it is a pointer: struct thread_group_cputime { struct task_cputime totals; }; struct thread_group_cputime { struct task_cputime *totals; }; We also add a new task_cputime substructure directly to signal_struct, to cache the earliest expiration of process-wide timers, and task_cputime also replaces the it_*_expires fields of task_struct (used for earliest expiration of thread timers). The "thread_group_cputime" structure contains process-wide timers that are updated via account_user_time() and friends. In the non-SMP case the structure is a simple aggregator; unfortunately in the SMP case that simplicity was not achievable due to cache-line contention between CPUs (in one measured case performance was actually _worse_ on a 16-cpu system than the same test on a 4-cpu system, due to this contention). For SMP, the thread_group_cputime counters are maintained as a per-cpu structure allocated using alloc_percpu(). The timer functions update only the timer field in the structure corresponding to the running CPU, obtained using per_cpu_ptr(). We define a set of inline functions in sched.h that we use to maintain the thread_group_cputime structure and hide the differences between UP and SMP implementations from the rest of the kernel. The thread_group_cputime_init() function initializes the thread_group_cputime structure for the given task. The thread_group_cputime_alloc() is a no-op for UP; for SMP it calls the out-of-line function thread_group_cputime_alloc_smp() to allocate and fill in the per-cpu structures and fields. The thread_group_cputime_free() function, also a no-op for UP, in SMP frees the per-cpu structures. The thread_group_cputime_clone_thread() function (also a UP no-op) for SMP calls thread_group_cputime_alloc() if the per-cpu structures haven't yet been allocated. The thread_group_cputime() function fills the task_cputime structure it is passed with the contents of the thread_group_cputime fields; in UP it's that simple but in SMP it must also safely check that tsk->signal is non-NULL (if it is it just uses the appropriate fields of task_struct) and, if so, sums the per-cpu values for each online CPU. Finally, the three functions account_group_user_time(), account_group_system_time() and account_group_exec_runtime() are used by timer functions to update the respective fields of the thread_group_cputime structure. Non-SMP operation is trivial and will not be mentioned further. The per-cpu structure is always allocated when a task creates its first new thread, via a call to thread_group_cputime_clone_thread() from copy_signal(). It is freed at process exit via a call to thread_group_cputime_free() from cleanup_signal(). All functions that formerly summed utime/stime/sum_sched_runtime values from from all threads in the thread group now use thread_group_cputime() to snapshot the values in the thread_group_cputime structure or the values in the task structure itself if the per-cpu structure hasn't been allocated. Finally, the code in kernel/posix-cpu-timers.c has changed quite a bit. The run_posix_cpu_timers() function has been split into a fast path and a slow path; the former safely checks whether there are any expired thread timers and, if not, just returns, while the slow path does the heavy lifting. With the dedicated thread group fields, timers are no longer "rebalanced" and the process_timer_rebalance() function and related code has gone away. All summing loops are gone and all code that used them now uses the thread_group_cputime() inline. When process-wide timers are set, the new task_cputime structure in signal_struct is used to cache the earliest expiration; this is checked in the fast path. Performance The fix appears not to add significant overhead to existing operations. It generally performs the same as the current code except in two cases, one in which it performs slightly worse (Case 5 below) and one in which it performs very significantly better (Case 2 below). Overall it's a wash except in those two cases. I've since done somewhat more involved testing on a dual-core Opteron system. Case 1: With no itimer running, for a test with 100,000 threads, the fixed kernel took 1428.5 seconds, 513 seconds more than the unfixed system, all of which was spent in the system. There were twice as many voluntary context switches with the fix as without it. Case 2: With an itimer running at .01 second ticks and 4000 threads (the most an unmodified kernel can handle), the fixed kernel ran the test in eight percent of the time (5.8 seconds as opposed to 70 seconds) and had better tick accuracy (.012 seconds per tick as opposed to .023 seconds per tick). Case 3: A 4000-thread test with an initial timer tick of .01 second and an interval of 10,000 seconds (i.e. a timer that ticks only once) had very nearly the same performance in both cases: 6.3 seconds elapsed for the fixed kernel versus 5.5 seconds for the unfixed kernel. With fewer threads (eight in these tests), the Case 1 test ran in essentially the same time on both the modified and unmodified kernels (5.2 seconds versus 5.8 seconds). The Case 2 test ran in about the same time as well, 5.9 seconds versus 5.4 seconds but again with much better tick accuracy, .013 seconds per tick versus .025 seconds per tick for the unmodified kernel. Since the fix affected the rlimit code, I also tested soft and hard CPU limits. Case 4: With a hard CPU limit of 20 seconds and eight threads (and an itimer running), the modified kernel was very slightly favored in that while it killed the process in 19.997 seconds of CPU time (5.002 seconds of wall time), only .003 seconds of that was system time, the rest was user time. The unmodified kernel killed the process in 20.001 seconds of CPU (5.014 seconds of wall time) of which .016 seconds was system time. Really, though, the results were too close to call. The results were essentially the same with no itimer running. Case 5: With a soft limit of 20 seconds and a hard limit of 2000 seconds (where the hard limit would never be reached) and an itimer running, the modified kernel exhibited worse tick accuracy than the unmodified kernel: .050 seconds/tick versus .028 seconds/tick. Otherwise, performance was almost indistinguishable. With no itimer running this test exhibited virtually identical behavior and times in both cases. In times past I did some limited performance testing. those results are below. On a four-cpu Opteron system without this fix, a sixteen-thread test executed in 3569.991 seconds, of which user was 3568.435s and system was 1.556s. On the same system with the fix, user and elapsed time were about the same, but system time dropped to 0.007 seconds. Performance with eight, four and one thread were comparable. Interestingly, the timer ticks with the fix seemed more accurate: The sixteen-thread test with the fix received 149543 ticks for 0.024 seconds per tick, while the same test without the fix received 58720 for 0.061 seconds per tick. Both cases were configured for an interval of 0.01 seconds. Again, the other tests were comparable. Each thread in this test computed the primes up to 25,000,000. I also did a test with a large number of threads, 100,000 threads, which is impossible without the fix. In this case each thread computed the primes only up to 10,000 (to make the runtime manageable). System time dominated, at 1546.968 seconds out of a total 2176.906 seconds (giving a user time of 629.938s). It received 147651 ticks for 0.015 seconds per tick, still quite accurate. There is obviously no comparable test without the fix. Signed-off-by: Frank Mayhar Cc: Roland McGrath Cc: Alexey Dobriyan Cc: Andrew Morton Signed-off-by: Ingo Molnar --- security/selinux/hooks.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 03fc6a81ae32..69649783c266 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -75,6 +75,7 @@ #include #include #include +#include #include "avc.h" #include "objsec.h" @@ -2321,13 +2322,7 @@ static void selinux_bprm_post_apply_creds(struct linux_binprm *bprm) initrlim = init_task.signal->rlim+i; rlim->rlim_cur = min(rlim->rlim_max, initrlim->rlim_cur); } - if (current->signal->rlim[RLIMIT_CPU].rlim_cur != RLIM_INFINITY) { - /* - * This will cause RLIMIT_CPU calculations - * to be refigured. - */ - current->it_prof_expires = jiffies_to_cputime(1); - } + update_rlimit_cpu(rlim->rlim_cur); } /* Wake up the parent if it is waiting so that it can -- cgit v1.2.3 From 92562927826fceb2f8e69c89e28161b8c1e0b125 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Tue, 7 Oct 2008 14:00:12 -0400 Subject: integrity: special fs magic Discussion on the mailing list questioned the use of these magic values in userspace, concluding these values are already exported to userspace via statfs and their correct/incorrect usage is left up to the userspace application. - Move special fs magic number definitions to magic.h - Add magic.h include Signed-off-by: Mimi Zohar Reviewed-by: James Morris Signed-off-by: James Morris --- security/inode.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security') diff --git a/security/inode.c b/security/inode.c index ca4958ebad8d..efea5a605466 100644 --- a/security/inode.c +++ b/security/inode.c @@ -20,8 +20,7 @@ #include #include #include - -#define SECURITYFS_MAGIC 0x73636673 +#include static struct vfsmount *mount; static int mount_count; -- cgit v1.2.3 From 452a00d2ee288f2cbc36f676edd06cb14d2878c1 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Mon, 13 Oct 2008 10:39:13 +0100 Subject: tty: Make get_current_tty use a kref We now return a kref covered tty reference. That ensures the tty structure doesn't go away when you have a return from get_current_tty. This is not enough to protect you from most of the resources being freed behind your back - yet. [Updated to include fixes for SELinux problems found by Andrew Morton and an s390 leak found while debugging the former] Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- security/selinux/hooks.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4a7374c12d9c..089d61a23952 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2123,6 +2123,7 @@ static inline void flush_unauthorized_files(struct files_struct *files) mutex_lock(&tty_mutex); tty = get_current_tty(); + mutex_unlock(&tty_mutex); if (tty) { file_list_lock(); file = list_entry(tty->tty_files.next, typeof(*file), f_u.fu_list); @@ -2139,8 +2140,8 @@ static inline void flush_unauthorized_files(struct files_struct *files) } } file_list_unlock(); + tty_kref_put(tty); } - mutex_unlock(&tty_mutex); /* Reset controlling tty. */ if (drop_tty) no_tty(); -- cgit v1.2.3 From 934e6ebf96e8c1a0f299e64129fdaebc1132a427 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Mon, 13 Oct 2008 10:40:43 +0100 Subject: tty: Redo current tty locking Currently it is sometimes locked by the tty mutex and sometimes by the sighand lock. The latter is in fact correct and now we can hand back referenced objects we can fix this up without problems around sleeping functions. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- security/selinux/hooks.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 089d61a23952..48881394fbd4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2121,9 +2121,7 @@ static inline void flush_unauthorized_files(struct files_struct *files) long j = -1; int drop_tty = 0; - mutex_lock(&tty_mutex); tty = get_current_tty(); - mutex_unlock(&tty_mutex); if (tty) { file_list_lock(); file = list_entry(tty->tty_files.next, typeof(*file), f_u.fu_list); -- cgit v1.2.3 From a447c0932445f92ce6f4c1bd020f62c5097a7842 Mon Sep 17 00:00:00 2001 From: Steven Whitehouse Date: Mon, 13 Oct 2008 10:46:57 +0100 Subject: vfs: Use const for kernel parser table This is a much better version of a previous patch to make the parser tables constant. Rather than changing the typedef, we put the "const" in all the various places where its required, allowing the __initconst exception for nfsroot which was the cause of the previous trouble. This was posted for review some time ago and I believe its been in -mm since then. Signed-off-by: Steven Whitehouse Cc: Alexander Viro Signed-off-by: Linus Torvalds --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 88f19536efad..576e51199079 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -325,7 +325,7 @@ enum { Opt_rootcontext = 4, }; -static match_table_t tokens = { +static const match_table_t tokens = { {Opt_context, CONTEXT_STR "%s"}, {Opt_fscontext, FSCONTEXT_STR "%s"}, {Opt_defcontext, DEFCONTEXT_STR "%s"}, -- cgit v1.2.3 From 2cdc7241a290bb2b9ef4c2e2969a4a3ed92abb63 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Sat, 18 Oct 2008 20:28:06 -0700 Subject: devcgroup: use kmemdup() This saves 40 bytes on my x86_32 box. Signed-off-by: Li Zefan Acked-by: Serge Hallyn Cc: Paul Menage Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- security/device_cgroup.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 46f23971f7e4..9c625f65ee56 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -84,13 +84,9 @@ static int dev_whitelist_copy(struct list_head *dest, struct list_head *orig) struct dev_whitelist_item *wh, *tmp, *new; list_for_each_entry(wh, orig, list) { - new = kmalloc(sizeof(*wh), GFP_KERNEL); + new = kmemdup(wh, sizeof(*wh), GFP_KERNEL); if (!new) goto free_and_exit; - new->major = wh->major; - new->minor = wh->minor; - new->type = wh->type; - new->access = wh->access; list_add_tail(&new->list, dest); } @@ -114,11 +110,10 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, { struct dev_whitelist_item *whcopy, *walk; - whcopy = kmalloc(sizeof(*whcopy), GFP_KERNEL); + whcopy = kmemdup(wh, sizeof(*wh), GFP_KERNEL); if (!whcopy) return -ENOMEM; - memcpy(whcopy, wh, sizeof(*whcopy)); spin_lock(&dev_cgroup->lock); list_for_each_entry(walk, &dev_cgroup->whitelist, list) { if (walk->type != wh->type) -- cgit v1.2.3 From c012a54ae0b2ee2c73499f54596e0f5257288fec Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Sat, 18 Oct 2008 20:28:07 -0700 Subject: devcgroup: remove unused variable Signed-off-by: Li Zefan Acked-by: Serge Hallyn Cc: Paul Menage Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- security/device_cgroup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 9c625f65ee56..d5c15a7c67c3 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -355,7 +355,7 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, struct dev_cgroup *cur_devcgroup; const char *b; char *endp; - int retval = 0, count; + int count; struct dev_whitelist_item wh; if (!capable(CAP_SYS_ADMIN)) @@ -432,7 +432,6 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, } handle: - retval = 0; switch (filetype) { case DEVCG_ALLOW: if (!parent_has_perm(devcgroup, &wh)) -- cgit v1.2.3 From 47c59803becb55b72b26cdab3838d621a15badc8 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Sat, 18 Oct 2008 20:28:07 -0700 Subject: devcgroup: remove spin_lock() Since we introduced rcu for read side, spin_lock is used only for update. But we always hold cgroup_lock() when update, so spin_lock() is not need. Additional cleanup: 1) include linux/rcupdate.h explicitly 2) remove unused variable cur_devcgroup in devcgroup_update_access() Signed-off-by: Lai Jiangshan Acked-by: "Serge E. Hallyn" Cc: Paul Menage Cc: James Morris Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- security/device_cgroup.c | 34 +++++----------------------------- 1 file changed, 5 insertions(+), 29 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index d5c15a7c67c3..5ba78701adc3 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -1,5 +1,5 @@ /* - * dev_cgroup.c - device cgroup subsystem + * device_cgroup.c - device cgroup subsystem * * Copyright 2007 IBM Corp */ @@ -10,6 +10,7 @@ #include #include #include +#include #define ACC_MKNOD 1 #define ACC_READ 2 @@ -22,18 +23,8 @@ /* * whitelist locking rules: - * cgroup_lock() cannot be taken under dev_cgroup->lock. - * dev_cgroup->lock can be taken with or without cgroup_lock(). - * - * modifications always require cgroup_lock - * modifications to a list which is visible require the - * dev_cgroup->lock *and* cgroup_lock() - * walking the list requires dev_cgroup->lock or cgroup_lock(). - * - * reasoning: dev_whitelist_copy() needs to kmalloc, so needs - * a mutex, which the cgroup_lock() is. Since modifying - * a visible list requires both locks, either lock can be - * taken for walking the list. + * hold cgroup_lock() for update/read. + * hold rcu_read_lock() for read. */ struct dev_whitelist_item { @@ -47,7 +38,6 @@ struct dev_whitelist_item { struct dev_cgroup { struct cgroup_subsys_state css; struct list_head whitelist; - spinlock_t lock; }; static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) @@ -103,7 +93,6 @@ free_and_exit: /* Stupid prototype - don't bother combining existing entries */ /* * called under cgroup_lock() - * since the list is visible to other tasks, we need the spinlock also */ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, struct dev_whitelist_item *wh) @@ -114,7 +103,6 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, if (!whcopy) return -ENOMEM; - spin_lock(&dev_cgroup->lock); list_for_each_entry(walk, &dev_cgroup->whitelist, list) { if (walk->type != wh->type) continue; @@ -130,7 +118,6 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, if (whcopy != NULL) list_add_tail_rcu(&whcopy->list, &dev_cgroup->whitelist); - spin_unlock(&dev_cgroup->lock); return 0; } @@ -144,14 +131,12 @@ static void whitelist_item_free(struct rcu_head *rcu) /* * called under cgroup_lock() - * since the list is visible to other tasks, we need the spinlock also */ static void dev_whitelist_rm(struct dev_cgroup *dev_cgroup, struct dev_whitelist_item *wh) { struct dev_whitelist_item *walk, *tmp; - spin_lock(&dev_cgroup->lock); list_for_each_entry_safe(walk, tmp, &dev_cgroup->whitelist, list) { if (walk->type == DEV_ALL) goto remove; @@ -169,7 +154,6 @@ remove: call_rcu(&walk->rcu, whitelist_item_free); } } - spin_unlock(&dev_cgroup->lock); } /* @@ -209,7 +193,6 @@ static struct cgroup_subsys_state *devcgroup_create(struct cgroup_subsys *ss, } } - spin_lock_init(&dev_cgroup->lock); return &dev_cgroup->css; } @@ -325,15 +308,11 @@ static int parent_has_perm(struct dev_cgroup *childcg, { struct cgroup *pcg = childcg->css.cgroup->parent; struct dev_cgroup *parent; - int ret; if (!pcg) return 1; parent = cgroup_to_devcgroup(pcg); - spin_lock(&parent->lock); - ret = may_access_whitelist(parent, wh); - spin_unlock(&parent->lock); - return ret; + return may_access_whitelist(parent, wh); } /* @@ -352,7 +331,6 @@ static int parent_has_perm(struct dev_cgroup *childcg, static int devcgroup_update_access(struct dev_cgroup *devcgroup, int filetype, const char *buffer) { - struct dev_cgroup *cur_devcgroup; const char *b; char *endp; int count; @@ -361,8 +339,6 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, if (!capable(CAP_SYS_ADMIN)) return -EPERM; - cur_devcgroup = task_devcgroup(current); - memset(&wh, 0, sizeof(wh)); b = buffer; -- cgit v1.2.3 From 731572d39fcd3498702eda4600db4c43d51e0b26 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Wed, 29 Oct 2008 14:01:20 -0700 Subject: nfsd: fix vm overcommit crash Junjiro R. Okajima reported a problem where knfsd crashes if you are using it to export shmemfs objects and run strict overcommit. In this situation the current->mm based modifier to the overcommit goes through a NULL pointer. We could simply check for NULL and skip the modifier but we've caught other real bugs in the past from mm being NULL here - cases where we did need a valid mm set up (eg the exec bug about a year ago). To preserve the checks and get the logic we want shuffle the checking around and add a new helper to the vm_ security wrappers Also fix a current->mm reference in nommu that should use the passed mm [akpm@linux-foundation.org: coding-style fixes] [akpm@linux-foundation.org: fix build] Reported-by: Junjiro R. Okajima Acked-by: James Morris Signed-off-by: Alan Cox Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- security/security.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'security') diff --git a/security/security.c b/security/security.c index 255b08559b2b..c0acfa7177e5 100644 --- a/security/security.c +++ b/security/security.c @@ -198,14 +198,23 @@ int security_settime(struct timespec *ts, struct timezone *tz) int security_vm_enough_memory(long pages) { + WARN_ON(current->mm == NULL); return security_ops->vm_enough_memory(current->mm, pages); } int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) { + WARN_ON(mm == NULL); return security_ops->vm_enough_memory(mm, pages); } +int security_vm_enough_memory_kern(long pages) +{ + /* If current->mm is a kernel thread then we will pass NULL, + for this specific case that is fine */ + return security_ops->vm_enough_memory(current->mm, pages); +} + int security_bprm_alloc(struct linux_binprm *bprm) { return security_ops->bprm_alloc_security(bprm); -- cgit v1.2.3 From 37dd0bd04a3240d2922786d501e2f12cec858fbf Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 31 Oct 2008 17:40:00 -0400 Subject: SELinux: properly handle empty tty_files list SELinux has wrongly (since 2004) had an incorrect test for an empty tty->tty_files list. With an empty list selinux would be pointing to part of the tty struct itself and would then proceed to dereference that value and again dereference that result. An F10 change to plymouth on a ppc64 system is actually currently triggering this bug. This patch uses list_empty() to handle empty lists rather than looking at a meaningless location. [note, this fixes the oops reported in https://bugzilla.redhat.com/show_bug.cgi?id=469079] Signed-off-by: Eric Paris Signed-off-by: James Morris --- security/selinux/hooks.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 3e3fde7c1d2b..f85597a4d733 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2126,14 +2126,16 @@ static inline void flush_unauthorized_files(struct files_struct *files) tty = get_current_tty(); if (tty) { file_list_lock(); - file = list_entry(tty->tty_files.next, typeof(*file), f_u.fu_list); - if (file) { + if (!list_empty(&tty->tty_files)) { + struct inode *inode; + /* Revalidate access to controlling tty. Use inode_has_perm on the tty inode directly rather than using file_has_perm, as this particular open file may belong to another process and we are only interested in the inode-based check here. */ - struct inode *inode = file->f_path.dentry->d_inode; + file = list_first_entry(&tty->tty_files, struct file, f_u.fu_list); + inode = file->f_path.dentry->d_inode; if (inode_has_perm(current, inode, FILE__READ | FILE__WRITE, NULL)) { drop_tty = 1; -- cgit v1.2.3 From 3318a386e4ca68c76e0294363d29bdc46fcad670 Mon Sep 17 00:00:00 2001 From: Serge Hallyn Date: Thu, 30 Oct 2008 11:52:23 -0500 Subject: file caps: always start with clear bprm->caps_* While Linux doesn't honor setuid on scripts. However, it mistakenly behaves differently for file capabilities. This patch fixes that behavior by making sure that get_file_caps() begins with empty bprm->caps_*. That way when a script is loaded, its bprm->caps_* may be filled when binfmt_misc calls prepare_binprm(), but they will be cleared again when binfmt_elf calls prepare_binprm() next to read the interpreter's file capabilities. Signed-off-by: Serge Hallyn Acked-by: David Howells Acked-by: Andrew G. Morgan Signed-off-by: Linus Torvalds --- security/commoncap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 399bfdb9e2da..3976613db829 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -279,10 +279,10 @@ static int get_file_caps(struct linux_binprm *bprm) struct vfs_cap_data vcaps; struct inode *inode; - if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) { - bprm_clear_caps(bprm); + bprm_clear_caps(bprm); + + if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) return 0; - } dentry = dget(bprm->file->f_dentry); inode = dentry->d_inode; -- cgit v1.2.3