diff options
author | David Howells <dhowells@redhat.com> | 2013-12-02 06:24:18 -0500 |
---|---|---|
committer | David Howells <dhowells@redhat.com> | 2013-12-02 06:24:18 -0500 |
commit | 2480f57fb3023eb047c5f2d6dfefef41ab9b893c (patch) | |
tree | 65279528bb73307d236d8c614eb64dd6362e9e65 /security | |
parent | af91706d5ddecb4a9858cca9e90d463037cfd498 (diff) |
KEYS: Pre-clear struct key on allocation
The second word of key->payload does not get initialised in key_alloc(), but
the big_key type is relying on it having been cleared. The problem comes when
big_key fails to instantiate a large key and doesn't then set the payload. The
big_key_destroy() op is called from the garbage collector and this assumes that
the dentry pointer stored in the second word will be NULL if instantiation did
not complete.
Therefore just pre-clear the entire struct key on allocation rather than trying
to be clever and only initialising to 0 only those bits that aren't otherwise
initialised.
The lack of initialisation can lead to a bug report like the following if
big_key failed to initialise its file:
general protection fault: 0000 [#1] SMP
Modules linked in: ...
CPU: 0 PID: 51 Comm: kworker/0:1 Not tainted 3.10.0-53.el7.x86_64 #1
Hardware name: Dell Inc. PowerEdge 1955/0HC513, BIOS 1.4.4 12/09/2008
Workqueue: events key_garbage_collector
task: ffff8801294f5680 ti: ffff8801296e2000 task.ti: ffff8801296e2000
RIP: 0010:[<ffffffff811b4a51>] dput+0x21/0x2d0
...
Call Trace:
[<ffffffff811a7b06>] path_put+0x16/0x30
[<ffffffff81235604>] big_key_destroy+0x44/0x60
[<ffffffff8122dc4b>] key_gc_unused_keys.constprop.2+0x5b/0xe0
[<ffffffff8122df2f>] key_garbage_collector+0x1df/0x3c0
[<ffffffff8107759b>] process_one_work+0x17b/0x460
[<ffffffff8107834b>] worker_thread+0x11b/0x400
[<ffffffff81078230>] ? rescuer_thread+0x3e0/0x3e0
[<ffffffff8107eb00>] kthread+0xc0/0xd0
[<ffffffff8107ea40>] ? kthread_create_on_node+0x110/0x110
[<ffffffff815c4bec>] ret_from_fork+0x7c/0xb0
[<ffffffff8107ea40>] ? kthread_create_on_node+0x110/0x110
Reported-by: Patrik Kis <pkis@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Stephen Gallagher <sgallagh@redhat.com>
Diffstat (limited to 'security')
-rw-r--r-- | security/keys/key.c | 8 |
1 files changed, 1 insertions, 7 deletions
diff --git a/security/keys/key.c b/security/keys/key.c index 55d110f0aced..6e21c11e48bc 100644 --- a/security/keys/key.c +++ b/security/keys/key.c | |||
@@ -272,7 +272,7 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
272 | } | 272 | } |
273 | 273 | ||
274 | /* allocate and initialise the key and its description */ | 274 | /* allocate and initialise the key and its description */ |
275 | key = kmem_cache_alloc(key_jar, GFP_KERNEL); | 275 | key = kmem_cache_zalloc(key_jar, GFP_KERNEL); |
276 | if (!key) | 276 | if (!key) |
277 | goto no_memory_2; | 277 | goto no_memory_2; |
278 | 278 | ||
@@ -293,18 +293,12 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
293 | key->uid = uid; | 293 | key->uid = uid; |
294 | key->gid = gid; | 294 | key->gid = gid; |
295 | key->perm = perm; | 295 | key->perm = perm; |
296 | key->flags = 0; | ||
297 | key->expiry = 0; | ||
298 | key->payload.data = NULL; | ||
299 | key->security = NULL; | ||
300 | 296 | ||
301 | if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) | 297 | if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) |
302 | key->flags |= 1 << KEY_FLAG_IN_QUOTA; | 298 | key->flags |= 1 << KEY_FLAG_IN_QUOTA; |
303 | if (flags & KEY_ALLOC_TRUSTED) | 299 | if (flags & KEY_ALLOC_TRUSTED) |
304 | key->flags |= 1 << KEY_FLAG_TRUSTED; | 300 | key->flags |= 1 << KEY_FLAG_TRUSTED; |
305 | 301 | ||
306 | memset(&key->type_data, 0, sizeof(key->type_data)); | ||
307 | |||
308 | #ifdef KEY_DEBUGGING | 302 | #ifdef KEY_DEBUGGING |
309 | key->magic = KEY_DEBUG_MAGIC; | 303 | key->magic = KEY_DEBUG_MAGIC; |
310 | #endif | 304 | #endif |