lkml.org 
[lkml]   [2008]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/3] KEYS: Don't generate user and user session keyrings unless they're accessed
On Thu, 13 Mar 2008 19:14:37 +0000
David Howells <dhowells@redhat.com> wrote:

> Don't generate the per-UID user and user session keyrings unless they're
> explicitly accessed. This solves a problem during a login process whereby
> set*uid() is called before the SELinux PAM module, resulting in the per-UID
> keyrings having the wrong security labels.
>
> This also cures the problem of multiple per-UID keyrings sometimes appearing
> due to PAM modules (including pam_keyinit) setuiding and causing user_structs
> to come into and go out of existence whilst the session keyring pins the user
> keyring. This is achieved by first searching for extant per-UID keyrings before
> inventing new ones.
>
> The serial bound argument is also dropped from find_keyring_by_name() as it's
> not currently made use of (setting it to 0 disables the feature).
>
> ..
>
> -/* Initial keyrings */
> -extern struct key root_user_keyring;
> -extern struct key root_session_keyring;

hm, I didn't realise that the keys code had special knowlege of "root".
How does that play alongside the containers stuff?

> --- a/kernel/user.c
> +++ b/kernel/user.c
> ...
> +#ifdef CONFIG_KEYS
> + new->uid_keyring = new->session_keyring = NULL;
> +#endif

new->uid_keyring = NULL;
new->session_keyring = NULL;

would be more conventional.

But better would be to teach alloc_uid() about kmem_cache_zalloc() then
take a chainsaw to it.

It's sorely tempting to say that initialising an atomic_t with memset(0) is
OK. Heck, if it ever becomes not OK then we're screwed anwyay, because
vast tracts of code assumes that atomic_set(uninitalised_atomic, 0) works
OK.


I'll queue this up:


From: Andrew Morton <akpm@linux-foundation.org>

Use kmem_cache_zalloc(), remove large amounts of initialsiation code and
ifdeffery.

Note: this assumes that memset(*atomic_t, 0) correctly initialises the
atomic_t. This is true for all present archtiectures and if it becomes false
for a future architecture then we'll need to make large changes all over the
place anyway.


Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

kernel/user.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)

diff -puN kernel/user.c~alloc_uid-cleanup kernel/user.c
--- a/kernel/user.c~alloc_uid-cleanup
+++ a/kernel/user.c
@@ -356,7 +356,7 @@ void free_uid(struct user_struct *up)
local_irq_restore(flags);
}

-struct user_struct * alloc_uid(struct user_namespace *ns, uid_t uid)
+struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
{
struct hlist_head *hashent = uidhashentry(ns, uid);
struct user_struct *up, *new;
@@ -371,26 +371,12 @@ struct user_struct * alloc_uid(struct us
spin_unlock_irq(&uidhash_lock);

if (!up) {
- new = kmem_cache_alloc(uid_cachep, GFP_KERNEL);
+ new = kmem_cache_zalloc(uid_cachep, GFP_KERNEL);
if (!new)
goto out_unlock;

new->uid = uid;
atomic_set(&new->__count, 1);
- atomic_set(&new->processes, 0);
- atomic_set(&new->files, 0);
- atomic_set(&new->sigpending, 0);
-#ifdef CONFIG_INOTIFY_USER
- atomic_set(&new->inotify_watches, 0);
- atomic_set(&new->inotify_devs, 0);
-#endif
-#ifdef CONFIG_POSIX_MQUEUE
- new->mq_bytes = 0;
-#endif
- new->locked_shm = 0;
-#ifdef CONFIG_KEYS
- new->uid_keyring = new->session_keyring = NULL;
-#endif

if (sched_create_user(new) < 0)
goto out_free_user;
_


\
 
 \ /
  Last update: 2008-03-13 23:25    [W:1.024 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site