* [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace @ 2020-03-13 15:20 Waiman Long 2020-03-13 15:21 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore Waiman Long ` (4 more replies) 0 siblings, 5 replies; 22+ messages in thread From: Waiman Long @ 2020-03-13 15:20 UTC (permalink / raw) To: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar Cc: keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen, Waiman Long v3: - Reorganize the keyctl_read_key() code to make it more readable as suggested by Jarkko Sakkinen. - Add patch 3 to use kvmalloc() for safer large buffer allocation as suggested by David Howells. v2: - Handle NULL buffer and buflen properly in patch 1. - Fix a bug in big_key.c. - Add patch 2 to handle arbitrary large user-supplied buflen. The current security key read methods are called with the key semaphore held. The methods then copy out the key data to userspace which is subjected to page fault and may acquire the mmap semaphore. That can result in circular lock dependency and hence a chance to get into deadlock. To avoid such a deadlock, an internal buffer is now allocated for getting out the necessary data first. After releasing the key semaphore, the key data are then copied out to userspace sidestepping the circular lock dependency. The keyutils test suite was run and the test passed with these patchset applied without any falure. Waiman Long (3): KEYS: Don't write out to userspace while holding key semaphore KEYS: Avoid false positive ENOMEM error on key read KEYS: Use kvmalloc() to better handle large buffer allocation include/linux/key-type.h | 2 +- security/keys/big_key.c | 11 ++- security/keys/encrypted-keys/encrypted.c | 7 +- security/keys/internal.h | 14 ++++ security/keys/keyctl.c | 87 ++++++++++++++++++++--- security/keys/keyring.c | 6 +- security/keys/request_key_auth.c | 7 +- security/keys/trusted-keys/trusted_tpm1.c | 14 +--- security/keys/user_defined.c | 5 +- 9 files changed, 107 insertions(+), 46 deletions(-) -- 2.18.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-13 15:20 [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace Waiman Long @ 2020-03-13 15:21 ` Waiman Long 2020-03-15 19:21 ` Jarkko Sakkinen 2020-03-13 15:21 ` [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read Waiman Long ` (3 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Waiman Long @ 2020-03-13 15:21 UTC (permalink / raw) To: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar Cc: keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen, Waiman Long A lockdep circular locking dependency report was seen when running a keyutils test: [12537.027242] ====================================================== [12537.059309] WARNING: possible circular locking dependency detected [12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE --------- - - [12537.125253] ------------------------------------------------------ [12537.153189] keyctl/25598 is trying to acquire lock: [12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0 [12537.208365] [12537.208365] but task is already holding lock: [12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220 [12537.270476] [12537.270476] which lock already depends on the new lock. [12537.270476] [12537.307209] [12537.307209] the existing dependency chain (in reverse order) is: [12537.340754] [12537.340754] -> #3 (&type->lock_class){++++}: [12537.367434] down_write+0x4d/0x110 [12537.385202] __key_link_begin+0x87/0x280 [12537.405232] request_key_and_link+0x483/0xf70 [12537.427221] request_key+0x3c/0x80 [12537.444839] dns_query+0x1db/0x5a5 [dns_resolver] [12537.468445] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs] [12537.496731] cifs_reconnect+0xe04/0x2500 [cifs] [12537.519418] cifs_readv_from_socket+0x461/0x690 [cifs] [12537.546263] cifs_read_from_socket+0xa0/0xe0 [cifs] [12537.573551] cifs_demultiplex_thread+0x311/0x2db0 [cifs] [12537.601045] kthread+0x30c/0x3d0 [12537.617906] ret_from_fork+0x3a/0x50 [12537.636225] [12537.636225] -> #2 (root_key_user.cons_lock){+.+.}: [12537.664525] __mutex_lock+0x105/0x11f0 [12537.683734] request_key_and_link+0x35a/0xf70 [12537.705640] request_key+0x3c/0x80 [12537.723304] dns_query+0x1db/0x5a5 [dns_resolver] [12537.746773] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs] [12537.775607] cifs_reconnect+0xe04/0x2500 [cifs] [12537.798322] cifs_readv_from_socket+0x461/0x690 [cifs] [12537.823369] cifs_read_from_socket+0xa0/0xe0 [cifs] [12537.847262] cifs_demultiplex_thread+0x311/0x2db0 [cifs] [12537.873477] kthread+0x30c/0x3d0 [12537.890281] ret_from_fork+0x3a/0x50 [12537.908649] [12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}: [12537.935225] __mutex_lock+0x105/0x11f0 [12537.954450] cifs_call_async+0x102/0x7f0 [cifs] [12537.977250] smb2_async_readv+0x6c3/0xc90 [cifs] [12538.000659] cifs_readpages+0x120a/0x1e50 [cifs] [12538.023920] read_pages+0xf5/0x560 [12538.041583] __do_page_cache_readahead+0x41d/0x4b0 [12538.067047] ondemand_readahead+0x44c/0xc10 [12538.092069] filemap_fault+0xec1/0x1830 [12538.111637] __do_fault+0x82/0x260 [12538.129216] do_fault+0x419/0xfb0 [12538.146390] __handle_mm_fault+0x862/0xdf0 [12538.167408] handle_mm_fault+0x154/0x550 [12538.187401] __do_page_fault+0x42f/0xa60 [12538.207395] do_page_fault+0x38/0x5e0 [12538.225777] page_fault+0x1e/0x30 [12538.243010] [12538.243010] -> #0 (&mm->mmap_sem){++++}: [12538.267875] lock_acquire+0x14c/0x420 [12538.286848] __might_fault+0x119/0x1b0 [12538.306006] keyring_read_iterator+0x7e/0x170 [12538.327936] assoc_array_subtree_iterate+0x97/0x280 [12538.352154] keyring_read+0xe9/0x110 [12538.370558] keyctl_read_key+0x1b9/0x220 [12538.391470] do_syscall_64+0xa5/0x4b0 [12538.410511] entry_SYSCALL_64_after_hwframe+0x6a/0xdf [12538.435535] [12538.435535] other info that might help us debug this: [12538.435535] [12538.472829] Chain exists of: [12538.472829] &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class [12538.472829] [12538.524820] Possible unsafe locking scenario: [12538.524820] [12538.551431] CPU0 CPU1 [12538.572654] ---- ---- [12538.595865] lock(&type->lock_class); [12538.613737] lock(root_key_user.cons_lock); [12538.644234] lock(&type->lock_class); [12538.672410] lock(&mm->mmap_sem); [12538.687758] [12538.687758] *** DEADLOCK *** [12538.687758] [12538.714455] 1 lock held by keyctl/25598: [12538.732097] #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220 [12538.770573] [12538.770573] stack backtrace: [12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G [12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015 [12538.881963] Call Trace: [12538.892897] dump_stack+0x9a/0xf0 [12538.907908] print_circular_bug.isra.25.cold.50+0x1bc/0x279 [12538.932891] ? save_trace+0xd6/0x250 [12538.948979] check_prev_add.constprop.32+0xc36/0x14f0 [12538.971643] ? keyring_compare_object+0x104/0x190 [12538.992738] ? check_usage+0x550/0x550 [12539.009845] ? sched_clock+0x5/0x10 [12539.025484] ? sched_clock_cpu+0x18/0x1e0 [12539.043555] __lock_acquire+0x1f12/0x38d0 [12539.061551] ? trace_hardirqs_on+0x10/0x10 [12539.080554] lock_acquire+0x14c/0x420 [12539.100330] ? __might_fault+0xc4/0x1b0 [12539.119079] __might_fault+0x119/0x1b0 [12539.135869] ? __might_fault+0xc4/0x1b0 [12539.153234] keyring_read_iterator+0x7e/0x170 [12539.172787] ? keyring_read+0x110/0x110 [12539.190059] assoc_array_subtree_iterate+0x97/0x280 [12539.211526] keyring_read+0xe9/0x110 [12539.227561] ? keyring_gc_check_iterator+0xc0/0xc0 [12539.249076] keyctl_read_key+0x1b9/0x220 [12539.266660] do_syscall_64+0xa5/0x4b0 [12539.283091] entry_SYSCALL_64_after_hwframe+0x6a/0xdf One way to prevent this deadlock scenario from happening is to not allow writing to userspace while holding the key semaphore. Instead, an internal buffer is allocated for getting the keys out from the read method first before copying them out to userspace without holding the lock. That requires taking out the __user modifier from the read methods as well as additional changes to not use any userspace write helpers. Signed-off-by: Waiman Long <longman@redhat.com> --- include/linux/key-type.h | 2 +- security/keys/big_key.c | 11 ++--- security/keys/encrypted-keys/encrypted.c | 7 ++- security/keys/keyctl.c | 57 +++++++++++++++++++---- security/keys/keyring.c | 6 +-- security/keys/request_key_auth.c | 7 ++- security/keys/trusted-keys/trusted_tpm1.c | 14 +----- security/keys/user_defined.c | 5 +- 8 files changed, 65 insertions(+), 44 deletions(-) diff --git a/include/linux/key-type.h b/include/linux/key-type.h index 4ded94bcf274..2ab2d6d6aeab 100644 --- a/include/linux/key-type.h +++ b/include/linux/key-type.h @@ -127,7 +127,7 @@ struct key_type { * much is copied into the buffer * - shouldn't do the copy if the buffer is NULL */ - long (*read)(const struct key *key, char __user *buffer, size_t buflen); + long (*read)(const struct key *key, char *buffer, size_t buflen); /* handle request_key() for this type instead of invoking * /sbin/request-key (optional) diff --git a/security/keys/big_key.c b/security/keys/big_key.c index 001abe530a0d..82008f900930 100644 --- a/security/keys/big_key.c +++ b/security/keys/big_key.c @@ -352,7 +352,7 @@ void big_key_describe(const struct key *key, struct seq_file *m) * read the key data * - the key's semaphore is read-locked */ -long big_key_read(const struct key *key, char __user *buffer, size_t buflen) +long big_key_read(const struct key *key, char *buffer, size_t buflen) { size_t datalen = (size_t)key->payload.data[big_key_len]; long ret; @@ -391,9 +391,8 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) ret = datalen; - /* copy decrypted data to user */ - if (copy_to_user(buffer, buf->virt, datalen) != 0) - ret = -EFAULT; + /* copy out decrypted data */ + memcpy(buffer, buf->virt, datalen); err_fput: fput(file); @@ -401,9 +400,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) big_key_free_buffer(buf); } else { ret = datalen; - if (copy_to_user(buffer, key->payload.data[big_key_data], - datalen) != 0) - ret = -EFAULT; + memcpy(buffer, key->payload.data[big_key_data], datalen); } return ret; diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 60720f58cbe0..f6797ba44bf7 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -902,14 +902,14 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep) } /* - * encrypted_read - format and copy the encrypted data to userspace + * encrypted_read - format and copy out the encrypted data * * The resulting datablob format is: * <master-key name> <decrypted data length> <encrypted iv> <encrypted data> * * On success, return to userspace the encrypted key datablob size. */ -static long encrypted_read(const struct key *key, char __user *buffer, +static long encrypted_read(const struct key *key, char *buffer, size_t buflen) { struct encrypted_key_payload *epayload; @@ -957,8 +957,7 @@ static long encrypted_read(const struct key *key, char __user *buffer, key_put(mkey); memzero_explicit(derived_key, sizeof(derived_key)); - if (copy_to_user(buffer, ascii_buf, asciiblob_len) != 0) - ret = -EFAULT; + memcpy(buffer, ascii_buf, asciiblob_len); kzfree(ascii_buf); return asciiblob_len; diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 9b898c969558..81f68e434b9f 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -797,6 +797,21 @@ long keyctl_keyring_search(key_serial_t ringid, return ret; } +/* + * Call the read method + */ +static long __keyctl_read_key(struct key *key, char *buffer, size_t buflen) +{ + long ret; + + down_read(&key->sem); + ret = key_validate(key); + if (ret == 0) + ret = key->type->read(key, buffer, buflen); + up_read(&key->sem); + return ret; +} + /* * Read a key's payload. * @@ -844,16 +859,42 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) /* the key is probably readable - now try to read it */ can_read_key: - ret = -EOPNOTSUPP; - if (key->type->read) { - /* Read the data with the semaphore held (since we might sleep) + if (!key->type->read) { + ret = -EOPNOTSUPP; + goto error2; + } + + if (!buffer || !buflen) { + /* Get the key length from the read method */ + ret = __keyctl_read_key(key, NULL, 0); + } else { + + /* + * Read the data with the semaphore held (since we might sleep) * to protect against the key being updated or revoked. + * + * Allocating a temporary buffer to hold the keys before + * transferring them to user buffer to avoid potential + * deadlock involving page fault and mmap_sem. + */ + char *tmpbuf = kmalloc(buflen, GFP_KERNEL); + + if (!tmpbuf) { + ret = -ENOMEM; + goto error2; + } + ret = __keyctl_read_key(key, tmpbuf, buflen); + + /* + * Read methods will just return the required length + * without any copying if the provided length isn't big + * enough. */ - down_read(&key->sem); - ret = key_validate(key); - if (ret == 0) - ret = key->type->read(key, buffer, buflen); - up_read(&key->sem); + if ((ret > 0) && (ret <= buflen)) { + if (copy_to_user(buffer, tmpbuf, ret)) + ret = -EFAULT; + } + kzfree(tmpbuf); } error2: diff --git a/security/keys/keyring.c b/security/keys/keyring.c index febf36c6ddc5..5ca620d31cd3 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -459,7 +459,6 @@ static int keyring_read_iterator(const void *object, void *data) { struct keyring_read_iterator_context *ctx = data; const struct key *key = keyring_ptr_to_key(object); - int ret; kenter("{%s,%d},,{%zu/%zu}", key->type->name, key->serial, ctx->count, ctx->buflen); @@ -467,10 +466,7 @@ static int keyring_read_iterator(const void *object, void *data) if (ctx->count >= ctx->buflen) return 1; - ret = put_user(key->serial, ctx->buffer); - if (ret < 0) - return ret; - ctx->buffer++; + *ctx->buffer++ = key->serial; ctx->count += sizeof(key->serial); return 0; } diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c index ecba39c93fd9..41e9735006d0 100644 --- a/security/keys/request_key_auth.c +++ b/security/keys/request_key_auth.c @@ -22,7 +22,7 @@ static int request_key_auth_instantiate(struct key *, static void request_key_auth_describe(const struct key *, struct seq_file *); static void request_key_auth_revoke(struct key *); static void request_key_auth_destroy(struct key *); -static long request_key_auth_read(const struct key *, char __user *, size_t); +static long request_key_auth_read(const struct key *, char *, size_t); /* * The request-key authorisation key type definition. @@ -80,7 +80,7 @@ static void request_key_auth_describe(const struct key *key, * - the key's semaphore is read-locked */ static long request_key_auth_read(const struct key *key, - char __user *buffer, size_t buflen) + char *buffer, size_t buflen) { struct request_key_auth *rka = dereference_key_locked(key); size_t datalen; @@ -97,8 +97,7 @@ static long request_key_auth_read(const struct key *key, if (buflen > datalen) buflen = datalen; - if (copy_to_user(buffer, rka->callout_info, buflen) != 0) - ret = -EFAULT; + memcpy(buffer, rka->callout_info, buflen); } return ret; diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c index d2c5ec1e040b..8001ab07e63b 100644 --- a/security/keys/trusted-keys/trusted_tpm1.c +++ b/security/keys/trusted-keys/trusted_tpm1.c @@ -1130,11 +1130,10 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) * trusted_read - copy the sealed blob data to userspace in hex. * On success, return to userspace the trusted key datablob size. */ -static long trusted_read(const struct key *key, char __user *buffer, +static long trusted_read(const struct key *key, char *buffer, size_t buflen) { const struct trusted_key_payload *p; - char *ascii_buf; char *bufp; int i; @@ -1143,18 +1142,9 @@ static long trusted_read(const struct key *key, char __user *buffer, return -EINVAL; if (buffer && buflen >= 2 * p->blob_len) { - ascii_buf = kmalloc_array(2, p->blob_len, GFP_KERNEL); - if (!ascii_buf) - return -ENOMEM; - - bufp = ascii_buf; + bufp = buffer; for (i = 0; i < p->blob_len; i++) bufp = hex_byte_pack(bufp, p->blob[i]); - if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) { - kzfree(ascii_buf); - return -EFAULT; - } - kzfree(ascii_buf); } return 2 * p->blob_len; } diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c index 6f12de4ce549..07d4287e9084 100644 --- a/security/keys/user_defined.c +++ b/security/keys/user_defined.c @@ -168,7 +168,7 @@ EXPORT_SYMBOL_GPL(user_describe); * read the key data * - the key's semaphore is read-locked */ -long user_read(const struct key *key, char __user *buffer, size_t buflen) +long user_read(const struct key *key, char *buffer, size_t buflen) { const struct user_key_payload *upayload; long ret; @@ -181,8 +181,7 @@ long user_read(const struct key *key, char __user *buffer, size_t buflen) if (buflen > upayload->datalen) buflen = upayload->datalen; - if (copy_to_user(buffer, upayload->data, buflen) != 0) - ret = -EFAULT; + memcpy(buffer, upayload->data, buflen); } return ret; -- 2.18.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-13 15:21 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore Waiman Long @ 2020-03-15 19:21 ` Jarkko Sakkinen 2020-03-15 21:27 ` Jarkko Sakkinen ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Jarkko Sakkinen @ 2020-03-15 19:21 UTC (permalink / raw) To: Waiman Long Cc: David Howells, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On Fri, Mar 13, 2020 at 11:21:00AM -0400, Waiman Long wrote: > A lockdep circular locking dependency report was seen when running a > keyutils test: > > [12537.027242] ====================================================== > [12537.059309] WARNING: possible circular locking dependency detected > [12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE --------- - - > [12537.125253] ------------------------------------------------------ > [12537.153189] keyctl/25598 is trying to acquire lock: > [12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0 > [12537.208365] > [12537.208365] but task is already holding lock: > [12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220 > [12537.270476] > [12537.270476] which lock already depends on the new lock. > [12537.270476] > [12537.307209] > [12537.307209] the existing dependency chain (in reverse order) is: > [12537.340754] > [12537.340754] -> #3 (&type->lock_class){++++}: > [12537.367434] down_write+0x4d/0x110 > [12537.385202] __key_link_begin+0x87/0x280 > [12537.405232] request_key_and_link+0x483/0xf70 > [12537.427221] request_key+0x3c/0x80 > [12537.444839] dns_query+0x1db/0x5a5 [dns_resolver] > [12537.468445] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs] > [12537.496731] cifs_reconnect+0xe04/0x2500 [cifs] > [12537.519418] cifs_readv_from_socket+0x461/0x690 [cifs] > [12537.546263] cifs_read_from_socket+0xa0/0xe0 [cifs] > [12537.573551] cifs_demultiplex_thread+0x311/0x2db0 [cifs] > [12537.601045] kthread+0x30c/0x3d0 > [12537.617906] ret_from_fork+0x3a/0x50 > [12537.636225] > [12537.636225] -> #2 (root_key_user.cons_lock){+.+.}: > [12537.664525] __mutex_lock+0x105/0x11f0 > [12537.683734] request_key_and_link+0x35a/0xf70 > [12537.705640] request_key+0x3c/0x80 > [12537.723304] dns_query+0x1db/0x5a5 [dns_resolver] > [12537.746773] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs] > [12537.775607] cifs_reconnect+0xe04/0x2500 [cifs] > [12537.798322] cifs_readv_from_socket+0x461/0x690 [cifs] > [12537.823369] cifs_read_from_socket+0xa0/0xe0 [cifs] > [12537.847262] cifs_demultiplex_thread+0x311/0x2db0 [cifs] > [12537.873477] kthread+0x30c/0x3d0 > [12537.890281] ret_from_fork+0x3a/0x50 > [12537.908649] > [12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}: > [12537.935225] __mutex_lock+0x105/0x11f0 > [12537.954450] cifs_call_async+0x102/0x7f0 [cifs] > [12537.977250] smb2_async_readv+0x6c3/0xc90 [cifs] > [12538.000659] cifs_readpages+0x120a/0x1e50 [cifs] > [12538.023920] read_pages+0xf5/0x560 > [12538.041583] __do_page_cache_readahead+0x41d/0x4b0 > [12538.067047] ondemand_readahead+0x44c/0xc10 > [12538.092069] filemap_fault+0xec1/0x1830 > [12538.111637] __do_fault+0x82/0x260 > [12538.129216] do_fault+0x419/0xfb0 > [12538.146390] __handle_mm_fault+0x862/0xdf0 > [12538.167408] handle_mm_fault+0x154/0x550 > [12538.187401] __do_page_fault+0x42f/0xa60 > [12538.207395] do_page_fault+0x38/0x5e0 > [12538.225777] page_fault+0x1e/0x30 > [12538.243010] > [12538.243010] -> #0 (&mm->mmap_sem){++++}: > [12538.267875] lock_acquire+0x14c/0x420 > [12538.286848] __might_fault+0x119/0x1b0 > [12538.306006] keyring_read_iterator+0x7e/0x170 > [12538.327936] assoc_array_subtree_iterate+0x97/0x280 > [12538.352154] keyring_read+0xe9/0x110 > [12538.370558] keyctl_read_key+0x1b9/0x220 > [12538.391470] do_syscall_64+0xa5/0x4b0 > [12538.410511] entry_SYSCALL_64_after_hwframe+0x6a/0xdf > [12538.435535] > [12538.435535] other info that might help us debug this: > [12538.435535] > [12538.472829] Chain exists of: > [12538.472829] &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class > [12538.472829] > [12538.524820] Possible unsafe locking scenario: > [12538.524820] > [12538.551431] CPU0 CPU1 > [12538.572654] ---- ---- > [12538.595865] lock(&type->lock_class); > [12538.613737] lock(root_key_user.cons_lock); > [12538.644234] lock(&type->lock_class); > [12538.672410] lock(&mm->mmap_sem); > [12538.687758] > [12538.687758] *** DEADLOCK *** > [12538.687758] > [12538.714455] 1 lock held by keyctl/25598: > [12538.732097] #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220 > [12538.770573] > [12538.770573] stack backtrace: > [12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G > [12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015 > [12538.881963] Call Trace: > [12538.892897] dump_stack+0x9a/0xf0 > [12538.907908] print_circular_bug.isra.25.cold.50+0x1bc/0x279 > [12538.932891] ? save_trace+0xd6/0x250 > [12538.948979] check_prev_add.constprop.32+0xc36/0x14f0 > [12538.971643] ? keyring_compare_object+0x104/0x190 > [12538.992738] ? check_usage+0x550/0x550 > [12539.009845] ? sched_clock+0x5/0x10 > [12539.025484] ? sched_clock_cpu+0x18/0x1e0 > [12539.043555] __lock_acquire+0x1f12/0x38d0 > [12539.061551] ? trace_hardirqs_on+0x10/0x10 > [12539.080554] lock_acquire+0x14c/0x420 > [12539.100330] ? __might_fault+0xc4/0x1b0 > [12539.119079] __might_fault+0x119/0x1b0 > [12539.135869] ? __might_fault+0xc4/0x1b0 > [12539.153234] keyring_read_iterator+0x7e/0x170 > [12539.172787] ? keyring_read+0x110/0x110 > [12539.190059] assoc_array_subtree_iterate+0x97/0x280 > [12539.211526] keyring_read+0xe9/0x110 > [12539.227561] ? keyring_gc_check_iterator+0xc0/0xc0 > [12539.249076] keyctl_read_key+0x1b9/0x220 > [12539.266660] do_syscall_64+0xa5/0x4b0 > [12539.283091] entry_SYSCALL_64_after_hwframe+0x6a/0xdf > > One way to prevent this deadlock scenario from happening is to not > allow writing to userspace while holding the key semaphore. Instead, > an internal buffer is allocated for getting the keys out from the > read method first before copying them out to userspace without holding > the lock. > > That requires taking out the __user modifier from the read methods as > well as additional changes to not use any userspace write helpers. > > Signed-off-by: Waiman Long <longman@redhat.com> > --- > include/linux/key-type.h | 2 +- > security/keys/big_key.c | 11 ++--- > security/keys/encrypted-keys/encrypted.c | 7 ++- > security/keys/keyctl.c | 57 +++++++++++++++++++---- > security/keys/keyring.c | 6 +-- > security/keys/request_key_auth.c | 7 ++- > security/keys/trusted-keys/trusted_tpm1.c | 14 +----- > security/keys/user_defined.c | 5 +- > 8 files changed, 65 insertions(+), 44 deletions(-) > > diff --git a/include/linux/key-type.h b/include/linux/key-type.h > index 4ded94bcf274..2ab2d6d6aeab 100644 > --- a/include/linux/key-type.h > +++ b/include/linux/key-type.h > @@ -127,7 +127,7 @@ struct key_type { > * much is copied into the buffer > * - shouldn't do the copy if the buffer is NULL > */ > - long (*read)(const struct key *key, char __user *buffer, size_t buflen); > + long (*read)(const struct key *key, char *buffer, size_t buflen); > > /* handle request_key() for this type instead of invoking > * /sbin/request-key (optional) > diff --git a/security/keys/big_key.c b/security/keys/big_key.c > index 001abe530a0d..82008f900930 100644 > --- a/security/keys/big_key.c > +++ b/security/keys/big_key.c > @@ -352,7 +352,7 @@ void big_key_describe(const struct key *key, struct seq_file *m) > * read the key data > * - the key's semaphore is read-locked > */ > -long big_key_read(const struct key *key, char __user *buffer, size_t buflen) > +long big_key_read(const struct key *key, char *buffer, size_t buflen) > { > size_t datalen = (size_t)key->payload.data[big_key_len]; > long ret; > @@ -391,9 +391,8 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) > > ret = datalen; > > - /* copy decrypted data to user */ > - if (copy_to_user(buffer, buf->virt, datalen) != 0) > - ret = -EFAULT; > + /* copy out decrypted data */ > + memcpy(buffer, buf->virt, datalen); > > err_fput: > fput(file); > @@ -401,9 +400,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) > big_key_free_buffer(buf); > } else { > ret = datalen; > - if (copy_to_user(buffer, key->payload.data[big_key_data], > - datalen) != 0) > - ret = -EFAULT; > + memcpy(buffer, key->payload.data[big_key_data], datalen); > } > > return ret; > diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c > index 60720f58cbe0..f6797ba44bf7 100644 > --- a/security/keys/encrypted-keys/encrypted.c > +++ b/security/keys/encrypted-keys/encrypted.c > @@ -902,14 +902,14 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep) > } > > /* > - * encrypted_read - format and copy the encrypted data to userspace > + * encrypted_read - format and copy out the encrypted data > * > * The resulting datablob format is: > * <master-key name> <decrypted data length> <encrypted iv> <encrypted data> > * > * On success, return to userspace the encrypted key datablob size. > */ > -static long encrypted_read(const struct key *key, char __user *buffer, > +static long encrypted_read(const struct key *key, char *buffer, > size_t buflen) > { > struct encrypted_key_payload *epayload; > @@ -957,8 +957,7 @@ static long encrypted_read(const struct key *key, char __user *buffer, > key_put(mkey); > memzero_explicit(derived_key, sizeof(derived_key)); > > - if (copy_to_user(buffer, ascii_buf, asciiblob_len) != 0) > - ret = -EFAULT; > + memcpy(buffer, ascii_buf, asciiblob_len); > kzfree(ascii_buf); > > return asciiblob_len; > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c > index 9b898c969558..81f68e434b9f 100644 > --- a/security/keys/keyctl.c > +++ b/security/keys/keyctl.c > @@ -797,6 +797,21 @@ long keyctl_keyring_search(key_serial_t ringid, > return ret; > } > > +/* > + * Call the read method > + */ > +static long __keyctl_read_key(struct key *key, char *buffer, size_t buflen) > +{ > + long ret; > + > + down_read(&key->sem); > + ret = key_validate(key); > + if (ret == 0) > + ret = key->type->read(key, buffer, buflen); > + up_read(&key->sem); > + return ret; > +} > + > /* > * Read a key's payload. > * > @@ -844,16 +859,42 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) > > /* the key is probably readable - now try to read it */ > can_read_key: > - ret = -EOPNOTSUPP; > - if (key->type->read) { > - /* Read the data with the semaphore held (since we might sleep) > + if (!key->type->read) { > + ret = -EOPNOTSUPP; > + goto error2; > + } > + > + if (!buffer || !buflen) { > + /* Get the key length from the read method */ > + ret = __keyctl_read_key(key, NULL, 0); > + } else { > + > + /* > + * Read the data with the semaphore held (since we might sleep) > * to protect against the key being updated or revoked. > + * > + * Allocating a temporary buffer to hold the keys before > + * transferring them to user buffer to avoid potential > + * deadlock involving page fault and mmap_sem. > + */ > + char *tmpbuf = kmalloc(buflen, GFP_KERNEL); > + > + if (!tmpbuf) { > + ret = -ENOMEM; > + goto error2; > + } > + ret = __keyctl_read_key(key, tmpbuf, buflen); > + > + /* > + * Read methods will just return the required length > + * without any copying if the provided length isn't big > + * enough. > */ > - down_read(&key->sem); > - ret = key_validate(key); > - if (ret == 0) > - ret = key->type->read(key, buffer, buflen); > - up_read(&key->sem); > + if ((ret > 0) && (ret <= buflen)) { > + if (copy_to_user(buffer, tmpbuf, ret)) > + ret = -EFAULT; > + } > + kzfree(tmpbuf); > } > > error2: > diff --git a/security/keys/keyring.c b/security/keys/keyring.c > index febf36c6ddc5..5ca620d31cd3 100644 > --- a/security/keys/keyring.c > +++ b/security/keys/keyring.c > @@ -459,7 +459,6 @@ static int keyring_read_iterator(const void *object, void *data) > { > struct keyring_read_iterator_context *ctx = data; > const struct key *key = keyring_ptr_to_key(object); > - int ret; > > kenter("{%s,%d},,{%zu/%zu}", > key->type->name, key->serial, ctx->count, ctx->buflen); > @@ -467,10 +466,7 @@ static int keyring_read_iterator(const void *object, void *data) > if (ctx->count >= ctx->buflen) > return 1; > > - ret = put_user(key->serial, ctx->buffer); > - if (ret < 0) > - return ret; > - ctx->buffer++; > + *ctx->buffer++ = key->serial; > ctx->count += sizeof(key->serial); > return 0; > } > diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c > index ecba39c93fd9..41e9735006d0 100644 > --- a/security/keys/request_key_auth.c > +++ b/security/keys/request_key_auth.c > @@ -22,7 +22,7 @@ static int request_key_auth_instantiate(struct key *, > static void request_key_auth_describe(const struct key *, struct seq_file *); > static void request_key_auth_revoke(struct key *); > static void request_key_auth_destroy(struct key *); > -static long request_key_auth_read(const struct key *, char __user *, size_t); > +static long request_key_auth_read(const struct key *, char *, size_t); > > /* > * The request-key authorisation key type definition. > @@ -80,7 +80,7 @@ static void request_key_auth_describe(const struct key *key, > * - the key's semaphore is read-locked > */ > static long request_key_auth_read(const struct key *key, > - char __user *buffer, size_t buflen) > + char *buffer, size_t buflen) > { > struct request_key_auth *rka = dereference_key_locked(key); > size_t datalen; > @@ -97,8 +97,7 @@ static long request_key_auth_read(const struct key *key, > if (buflen > datalen) > buflen = datalen; > > - if (copy_to_user(buffer, rka->callout_info, buflen) != 0) > - ret = -EFAULT; > + memcpy(buffer, rka->callout_info, buflen); > } > > return ret; > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c > index d2c5ec1e040b..8001ab07e63b 100644 > --- a/security/keys/trusted-keys/trusted_tpm1.c > +++ b/security/keys/trusted-keys/trusted_tpm1.c > @@ -1130,11 +1130,10 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) > * trusted_read - copy the sealed blob data to userspace in hex. > * On success, return to userspace the trusted key datablob size. > */ > -static long trusted_read(const struct key *key, char __user *buffer, > +static long trusted_read(const struct key *key, char *buffer, > size_t buflen) > { > const struct trusted_key_payload *p; > - char *ascii_buf; > char *bufp; > int i; > > @@ -1143,18 +1142,9 @@ static long trusted_read(const struct key *key, char __user *buffer, > return -EINVAL; > > if (buffer && buflen >= 2 * p->blob_len) { > - ascii_buf = kmalloc_array(2, p->blob_len, GFP_KERNEL); > - if (!ascii_buf) > - return -ENOMEM; > - > - bufp = ascii_buf; > + bufp = buffer; > for (i = 0; i < p->blob_len; i++) > bufp = hex_byte_pack(bufp, p->blob[i]); > - if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) { > - kzfree(ascii_buf); > - return -EFAULT; > - } > - kzfree(ascii_buf); > } > return 2 * p->blob_len; > } > diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c > index 6f12de4ce549..07d4287e9084 100644 > --- a/security/keys/user_defined.c > +++ b/security/keys/user_defined.c > @@ -168,7 +168,7 @@ EXPORT_SYMBOL_GPL(user_describe); > * read the key data > * - the key's semaphore is read-locked > */ > -long user_read(const struct key *key, char __user *buffer, size_t buflen) > +long user_read(const struct key *key, char *buffer, size_t buflen) > { > const struct user_key_payload *upayload; > long ret; > @@ -181,8 +181,7 @@ long user_read(const struct key *key, char __user *buffer, size_t buflen) > if (buflen > upayload->datalen) > buflen = upayload->datalen; > > - if (copy_to_user(buffer, upayload->data, buflen) != 0) > - ret = -EFAULT; > + memcpy(buffer, upayload->data, buflen); > } > > return ret; > -- > 2.18.1 > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-15 19:21 ` Jarkko Sakkinen @ 2020-03-15 21:27 ` Jarkko Sakkinen 2020-03-16 11:22 ` David Howells 2020-03-16 11:34 ` David Howells 2 siblings, 0 replies; 22+ messages in thread From: Jarkko Sakkinen @ 2020-03-15 21:27 UTC (permalink / raw) To: Waiman Long, David Howells Cc: James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On Sun, Mar 15, 2020 at 09:21:16PM +0200, Jarkko Sakkinen wrote: > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> I guess we cannot sanely define fixes tag for this one, can we? Surely this should still go to stable. Do you have a test case that can reproduce this on a constant basis? /Jarkko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-15 19:21 ` Jarkko Sakkinen 2020-03-15 21:27 ` Jarkko Sakkinen @ 2020-03-16 11:22 ` David Howells 2020-03-16 13:53 ` Jarkko Sakkinen 2020-03-16 11:34 ` David Howells 2 siblings, 1 reply; 22+ messages in thread From: David Howells @ 2020-03-16 11:22 UTC (permalink / raw) To: Jarkko Sakkinen Cc: dhowells, Waiman Long, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > I guess we cannot sanely define fixes tag for this one, can we? Use: Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2") David ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-16 11:22 ` David Howells @ 2020-03-16 13:53 ` Jarkko Sakkinen 2020-03-16 16:33 ` Waiman Long 2020-03-17 18:10 ` Waiman Long 0 siblings, 2 replies; 22+ messages in thread From: Jarkko Sakkinen @ 2020-03-16 13:53 UTC (permalink / raw) To: David Howells Cc: Waiman Long, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On Mon, 2020-03-16 at 11:22 +0000, David Howells wrote: > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > > I guess we cannot sanely define fixes tag for this one, can we? > > Use: > > Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2") > > David Longmao, please include this to the next version. /Jarkko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-16 13:53 ` Jarkko Sakkinen @ 2020-03-16 16:33 ` Waiman Long 2020-03-17 18:10 ` Waiman Long 1 sibling, 0 replies; 22+ messages in thread From: Waiman Long @ 2020-03-16 16:33 UTC (permalink / raw) To: Jarkko Sakkinen, David Howells Cc: James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On 3/16/20 9:53 AM, Jarkko Sakkinen wrote: > On Mon, 2020-03-16 at 11:22 +0000, David Howells wrote: >> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: >> >>> I guess we cannot sanely define fixes tag for this one, can we? >> Use: >> >> Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2") >> >> David > Longmao, please include this to the next version. > > /Jarkko > Sure, will do. Cheers, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-16 13:53 ` Jarkko Sakkinen 2020-03-16 16:33 ` Waiman Long @ 2020-03-17 18:10 ` Waiman Long 1 sibling, 0 replies; 22+ messages in thread From: Waiman Long @ 2020-03-17 18:10 UTC (permalink / raw) To: Jarkko Sakkinen, David Howells Cc: James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On 3/16/20 9:53 AM, Jarkko Sakkinen wrote: > On Mon, 2020-03-16 at 11:22 +0000, David Howells wrote: >> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: >> >>> I guess we cannot sanely define fixes tag for this one, can we? >> Use: >> >> Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2") >> >> David > Longmao, please include this to the next version. > > /Jarkko > Sure, will do. Cheers, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-15 19:21 ` Jarkko Sakkinen 2020-03-15 21:27 ` Jarkko Sakkinen 2020-03-16 11:22 ` David Howells @ 2020-03-16 11:34 ` David Howells 2 siblings, 0 replies; 22+ messages in thread From: David Howells @ 2020-03-16 11:34 UTC (permalink / raw) To: Jarkko Sakkinen Cc: dhowells, Waiman Long, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > Do you have a test case that can reproduce this on a constant basis? In this case it's quite tricky because a network filesystem is involved. I wonder if it might be possible to do it with fscrypt or ecryptfs. David ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read 2020-03-13 15:20 [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace Waiman Long 2020-03-13 15:21 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore Waiman Long @ 2020-03-13 15:21 ` Waiman Long 2020-03-15 21:32 ` Jarkko Sakkinen 2020-03-13 15:21 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation Waiman Long ` (2 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Waiman Long @ 2020-03-13 15:21 UTC (permalink / raw) To: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar Cc: keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen, Waiman Long By allocating a kernel buffer with an user-supplied buffer length, it is possible that a false positive ENOMEM error may be returned because the user-supplied length is just too large even if the system do have enough memory to hold the actual key data. To reduce this possibility, we set a threshold (1024) over which we do check the actual key length first before allocating a buffer of the right size to hold it. Signed-off-by: Waiman Long <longman@redhat.com> --- security/keys/keyctl.c | 48 ++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 81f68e434b9f..a05a4dd2f9ce 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -877,24 +877,50 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) * transferring them to user buffer to avoid potential * deadlock involving page fault and mmap_sem. */ - char *tmpbuf = kmalloc(buflen, GFP_KERNEL); - - if (!tmpbuf) { - ret = -ENOMEM; - goto error2; - } - ret = __keyctl_read_key(key, tmpbuf, buflen); + char *tmpbuf = NULL; + size_t tmpbuflen = buflen; /* - * Read methods will just return the required length - * without any copying if the provided length isn't big - * enough. + * We don't want an erronous -ENOMEM error due to an + * arbitrary large user-supplied buflen. So if buflen + * exceeds a threshold (1024 bytes in this case), we call + * the read method twice. The first time to get the buffer + * length and the second time to read out the key data. + * + * N.B. All the read methods will return the required + * buffer length with a NULL input buffer or when + * the input buffer length isn't large enough. */ + if (buflen <= 0x400) { +allocbuf: + tmpbuf = kmalloc(tmpbuflen, GFP_KERNEL); + if (!tmpbuf) { + ret = -ENOMEM; + goto error2; + } + } + + ret = __keyctl_read_key(key, tmpbuf, tmpbuflen); if ((ret > 0) && (ret <= buflen)) { + /* + * It is possible, though unlikely, that the key + * changes in between the up_read->down_read period. + * If the key becomes longer, we will have to + * allocate a larger buffer and redo the key read + * again. + */ + if (!tmpbuf || unlikely(ret > tmpbuflen)) { + if (unlikely(tmpbuf)) + kzfree(tmpbuf); + tmpbuflen = ret; + goto allocbuf; + } + if (copy_to_user(buffer, tmpbuf, ret)) ret = -EFAULT; } - kzfree(tmpbuf); + if (tmpbuf) + kzfree(tmpbuf); } error2: -- 2.18.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read 2020-03-13 15:21 ` [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read Waiman Long @ 2020-03-15 21:32 ` Jarkko Sakkinen 2020-03-17 18:36 ` Waiman Long 0 siblings, 1 reply; 22+ messages in thread From: Jarkko Sakkinen @ 2020-03-15 21:32 UTC (permalink / raw) To: Waiman Long Cc: David Howells, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On Fri, Mar 13, 2020 at 11:21:01AM -0400, Waiman Long wrote: > - * Read methods will just return the required length > - * without any copying if the provided length isn't big > - * enough. > + * We don't want an erronous -ENOMEM error due to an > + * arbitrary large user-supplied buflen. So if buflen > + * exceeds a threshold (1024 bytes in this case), we call > + * the read method twice. The first time to get the buffer > + * length and the second time to read out the key data. > + * > + * N.B. All the read methods will return the required > + * buffer length with a NULL input buffer or when > + * the input buffer length isn't large enough. > */ > + if (buflen <= 0x400) { 1. The overwhelmingly long comment. Will be destined to rotten. 2. Magic number. 3. The cap must be updated both in comment and code, and not only that, but the numbers use a different base (dec and hex). /Jarkko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read 2020-03-15 21:32 ` Jarkko Sakkinen @ 2020-03-17 18:36 ` Waiman Long 0 siblings, 0 replies; 22+ messages in thread From: Waiman Long @ 2020-03-17 18:36 UTC (permalink / raw) To: Jarkko Sakkinen Cc: David Howells, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On 3/15/20 5:32 PM, Jarkko Sakkinen wrote: > On Fri, Mar 13, 2020 at 11:21:01AM -0400, Waiman Long wrote: >> - * Read methods will just return the required length >> - * without any copying if the provided length isn't big >> - * enough. >> + * We don't want an erronous -ENOMEM error due to an >> + * arbitrary large user-supplied buflen. So if buflen >> + * exceeds a threshold (1024 bytes in this case), we call >> + * the read method twice. The first time to get the buffer >> + * length and the second time to read out the key data. >> + * >> + * N.B. All the read methods will return the required >> + * buffer length with a NULL input buffer or when >> + * the input buffer length isn't large enough. >> */ >> + if (buflen <= 0x400) { > 1. The overwhelmingly long comment. Will be destined to rotten. > 2. Magic number. > 3. The cap must be updated both in comment and code, and not only > that, but the numbers use a different base (dec and hex). > > /Jarkko > Thank for the comment. I will make the necessary change. Cheers, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-13 15:20 [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace Waiman Long 2020-03-13 15:21 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore Waiman Long 2020-03-13 15:21 ` [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read Waiman Long @ 2020-03-13 15:21 ` Waiman Long 2020-03-13 16:43 ` Eric Biggers 2020-03-16 11:50 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore David Howells 2020-03-16 14:24 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation David Howells 4 siblings, 1 reply; 22+ messages in thread From: Waiman Long @ 2020-03-13 15:21 UTC (permalink / raw) To: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar Cc: keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen, Waiman Long For large multi-page temporary buffer allocation, the security/keys subsystem don't need contiguous physical pages. It will work perfectly fine with virtually mapped pages. Replace the kmalloc() call by kvmalloc() and provide a __kvzfree() helper function to clear and free the kvmalloc'ed buffer. This will reduce the chance of memory allocation failure just because of highly fragmented pages. Suggested-by: David Howells <dhowells@redhat.com> Signed-off-by: Waiman Long <longman@redhat.com> --- security/keys/internal.h | 14 ++++++++++++++ security/keys/keyctl.c | 10 +++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/security/keys/internal.h b/security/keys/internal.h index ba3e2da14cef..855b11eb73ee 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -16,6 +16,8 @@ #include <linux/keyctl.h> #include <linux/refcount.h> #include <linux/compat.h> +#include <linux/mm.h> +#include <linux/vmalloc.h> struct iovec; @@ -349,4 +351,16 @@ static inline void key_check(const struct key *key) #endif +/* + * Helper function to clear and free a kvmalloc'ed memory object. + */ +static inline void __kvzfree(const void *addr, size_t len) +{ + if (is_vmalloc_addr(addr)) { + memset((void *)addr, 0, len); + vfree(addr); + } else { + kzfree(addr); + } +} #endif /* _INTERNAL_H */ diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index a05a4dd2f9ce..878259cf35d5 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -339,7 +339,7 @@ long keyctl_update_key(key_serial_t id, payload = NULL; if (plen) { ret = -ENOMEM; - payload = kmalloc(plen, GFP_KERNEL); + payload = kvmalloc(plen, GFP_KERNEL); if (!payload) goto error; @@ -360,7 +360,7 @@ long keyctl_update_key(key_serial_t id, key_ref_put(key_ref); error2: - kzfree(payload); + __kvzfree(payload, plen); error: return ret; } @@ -893,7 +893,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) */ if (buflen <= 0x400) { allocbuf: - tmpbuf = kmalloc(tmpbuflen, GFP_KERNEL); + tmpbuf = kvmalloc(tmpbuflen, GFP_KERNEL); if (!tmpbuf) { ret = -ENOMEM; goto error2; @@ -911,7 +911,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) */ if (!tmpbuf || unlikely(ret > tmpbuflen)) { if (unlikely(tmpbuf)) - kzfree(tmpbuf); + __kvzfree(tmpbuf, tmpbuflen); tmpbuflen = ret; goto allocbuf; } @@ -920,7 +920,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) ret = -EFAULT; } if (tmpbuf) - kzfree(tmpbuf); + __kvzfree(tmpbuf, tmpbuflen); } error2: -- 2.18.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-13 15:21 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation Waiman Long @ 2020-03-13 16:43 ` Eric Biggers 2020-03-13 17:49 ` Waiman Long 0 siblings, 1 reply; 22+ messages in thread From: Eric Biggers @ 2020-03-13 16:43 UTC (permalink / raw) To: Waiman Long Cc: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Chris von Recklinghausen On Fri, Mar 13, 2020 at 11:21:02AM -0400, Waiman Long wrote: > For large multi-page temporary buffer allocation, the security/keys > subsystem don't need contiguous physical pages. It will work perfectly > fine with virtually mapped pages. > > Replace the kmalloc() call by kvmalloc() and provide a __kvzfree() > helper function to clear and free the kvmalloc'ed buffer. This will > reduce the chance of memory allocation failure just because of highly > fragmented pages. > > Suggested-by: David Howells <dhowells@redhat.com> > Signed-off-by: Waiman Long <longman@redhat.com> > --- > security/keys/internal.h | 14 ++++++++++++++ > security/keys/keyctl.c | 10 +++++----- > 2 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/security/keys/internal.h b/security/keys/internal.h > index ba3e2da14cef..855b11eb73ee 100644 > --- a/security/keys/internal.h > +++ b/security/keys/internal.h > @@ -16,6 +16,8 @@ > #include <linux/keyctl.h> > #include <linux/refcount.h> > #include <linux/compat.h> > +#include <linux/mm.h> > +#include <linux/vmalloc.h> > > struct iovec; > > @@ -349,4 +351,16 @@ static inline void key_check(const struct key *key) > > #endif > > +/* > + * Helper function to clear and free a kvmalloc'ed memory object. > + */ > +static inline void __kvzfree(const void *addr, size_t len) > +{ > + if (is_vmalloc_addr(addr)) { > + memset((void *)addr, 0, len); > + vfree(addr); > + } else { > + kzfree(addr); > + } > +} Since this takes the length as a parameter, it can be simplified to: static inline void __kvzfree(const void *addr, size_t len) { if (addr) { memset((void *)addr, 0, len); kvfree(addr); } } > if (!tmpbuf || unlikely(ret > tmpbuflen)) { > if (unlikely(tmpbuf)) > - kzfree(tmpbuf); > + __kvzfree(tmpbuf, tmpbuflen); Both kzfree() and __kvzfree() handle a NULL pointer, so there's no need for the NULL check first. > @@ -920,7 +920,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) > ret = -EFAULT; > } > if (tmpbuf) > - kzfree(tmpbuf); > + __kvzfree(tmpbuf, tmpbuflen); Likewise here. No need for the NULL check. - Eric ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-13 16:43 ` Eric Biggers @ 2020-03-13 17:49 ` Waiman Long 2020-03-15 21:52 ` Jarkko Sakkinen 0 siblings, 1 reply; 22+ messages in thread From: Waiman Long @ 2020-03-13 17:49 UTC (permalink / raw) To: Eric Biggers Cc: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Chris von Recklinghausen On 3/13/20 12:43 PM, Eric Biggers wrote: > On Fri, Mar 13, 2020 at 11:21:02AM -0400, Waiman Long wrote: >> For large multi-page temporary buffer allocation, the security/keys >> subsystem don't need contiguous physical pages. It will work perfectly >> fine with virtually mapped pages. >> >> Replace the kmalloc() call by kvmalloc() and provide a __kvzfree() >> helper function to clear and free the kvmalloc'ed buffer. This will >> reduce the chance of memory allocation failure just because of highly >> fragmented pages. >> >> Suggested-by: David Howells <dhowells@redhat.com> >> Signed-off-by: Waiman Long <longman@redhat.com> >> --- >> security/keys/internal.h | 14 ++++++++++++++ >> security/keys/keyctl.c | 10 +++++----- >> 2 files changed, 19 insertions(+), 5 deletions(-) >> >> diff --git a/security/keys/internal.h b/security/keys/internal.h >> index ba3e2da14cef..855b11eb73ee 100644 >> --- a/security/keys/internal.h >> +++ b/security/keys/internal.h >> @@ -16,6 +16,8 @@ >> #include <linux/keyctl.h> >> #include <linux/refcount.h> >> #include <linux/compat.h> >> +#include <linux/mm.h> >> +#include <linux/vmalloc.h> >> >> struct iovec; >> >> @@ -349,4 +351,16 @@ static inline void key_check(const struct key *key) >> >> #endif >> >> +/* >> + * Helper function to clear and free a kvmalloc'ed memory object. >> + */ >> +static inline void __kvzfree(const void *addr, size_t len) >> +{ >> + if (is_vmalloc_addr(addr)) { >> + memset((void *)addr, 0, len); >> + vfree(addr); >> + } else { >> + kzfree(addr); >> + } >> +} > Since this takes the length as a parameter, it can be simplified to: > > static inline void __kvzfree(const void *addr, size_t len) > { > if (addr) { > memset((void *)addr, 0, len); > kvfree(addr); > } > } Yes, that will work too. >> if (!tmpbuf || unlikely(ret > tmpbuflen)) { >> if (unlikely(tmpbuf)) >> - kzfree(tmpbuf); >> + __kvzfree(tmpbuf, tmpbuflen); > Both kzfree() and __kvzfree() handle a NULL pointer, so there's no need for the > NULL check first. > I would like to keep this one because of the unlikely annotation. >> @@ -920,7 +920,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) >> ret = -EFAULT; >> } >> if (tmpbuf) >> - kzfree(tmpbuf); >> + __kvzfree(tmpbuf, tmpbuflen); > Likewise here. No need for the NULL check. Yes, that tmpbuf check is not really necessary, but it doesn't harm either. My plan is to send out a mm patch to officially add the kvzfree() function to mm/util.c. I will remove the tmpbuf check at that time if you don't mind. Cheers, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-13 17:49 ` Waiman Long @ 2020-03-15 21:52 ` Jarkko Sakkinen 2020-03-15 22:01 ` Waiman Long 0 siblings, 1 reply; 22+ messages in thread From: Jarkko Sakkinen @ 2020-03-15 21:52 UTC (permalink / raw) To: Waiman Long Cc: Eric Biggers, David Howells, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Chris von Recklinghausen On Fri, Mar 13, 2020 at 01:49:57PM -0400, Waiman Long wrote: > >> if (!tmpbuf || unlikely(ret > tmpbuflen)) { > >> if (unlikely(tmpbuf)) > >> - kzfree(tmpbuf); > >> + __kvzfree(tmpbuf, tmpbuflen); > > Both kzfree() and __kvzfree() handle a NULL pointer, so there's no need for the > > NULL check first. > > > I would like to keep this one because of the unlikely annotation. What (measurable) gain does it bring anyway? /Jarkko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-15 21:52 ` Jarkko Sakkinen @ 2020-03-15 22:01 ` Waiman Long 0 siblings, 0 replies; 22+ messages in thread From: Waiman Long @ 2020-03-15 22:01 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Eric Biggers, David Howells, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Chris von Recklinghausen On 3/15/20 5:52 PM, Jarkko Sakkinen wrote: > On Fri, Mar 13, 2020 at 01:49:57PM -0400, Waiman Long wrote: >>>> if (!tmpbuf || unlikely(ret > tmpbuflen)) { >>>> if (unlikely(tmpbuf)) >>>> - kzfree(tmpbuf); >>>> + __kvzfree(tmpbuf, tmpbuflen); >>> Both kzfree() and __kvzfree() handle a NULL pointer, so there's no need for the >>> NULL check first. >>> >> I would like to keep this one because of the unlikely annotation. > What (measurable) gain does it bring anyway? It is not a performance issue. I just want to indicate that the need to free should not happen at all. It match the unlikely tag in the if condition above. Cheers, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-13 15:20 [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace Waiman Long ` (2 preceding siblings ...) 2020-03-13 15:21 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation Waiman Long @ 2020-03-16 11:50 ` David Howells 2020-03-17 18:09 ` Waiman Long 2020-03-16 14:24 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation David Howells 4 siblings, 1 reply; 22+ messages in thread From: David Howells @ 2020-03-16 11:50 UTC (permalink / raw) To: Waiman Long Cc: dhowells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen Waiman Long <longman@redhat.com> wrote: > include/linux/key-type.h | 2 +- > security/keys/big_key.c | 11 ++--- > security/keys/encrypted-keys/encrypted.c | 7 ++- > security/keys/keyctl.c | 57 +++++++++++++++++++---- > security/keys/keyring.c | 6 +-- > security/keys/request_key_auth.c | 7 ++- > security/keys/trusted-keys/trusted_tpm1.c | 14 +----- > security/keys/user_defined.c | 5 +- > ... > - long (*read)(const struct key *key, char __user *buffer, size_t buflen); > + long (*read)(const struct key *key, char *buffer, size_t buflen); Note that there are read functions outside of security/keys/ that also need fixing - dns_resolver_read() and rxrpc_read(). David ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore 2020-03-16 11:50 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore David Howells @ 2020-03-17 18:09 ` Waiman Long 0 siblings, 0 replies; 22+ messages in thread From: Waiman Long @ 2020-03-17 18:09 UTC (permalink / raw) To: David Howells Cc: Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On 3/16/20 7:50 AM, David Howells wrote: > Waiman Long <longman@redhat.com> wrote: > >> include/linux/key-type.h | 2 +- >> security/keys/big_key.c | 11 ++--- >> security/keys/encrypted-keys/encrypted.c | 7 ++- >> security/keys/keyctl.c | 57 +++++++++++++++++++---- >> security/keys/keyring.c | 6 +-- >> security/keys/request_key_auth.c | 7 ++- >> security/keys/trusted-keys/trusted_tpm1.c | 14 +----- >> security/keys/user_defined.c | 5 +- >> ... >> - long (*read)(const struct key *key, char __user *buffer, size_t buflen); >> + long (*read)(const struct key *key, char *buffer, size_t buflen); > Note that there are read functions outside of security/keys/ that also need > fixing - dns_resolver_read() and rxrpc_read(). > > David Yes, I am going to fix that. Sorry for the delay as I am juggling a few different tasks. Cheers, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-13 15:20 [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace Waiman Long ` (3 preceding siblings ...) 2020-03-16 11:50 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore David Howells @ 2020-03-16 14:24 ` David Howells 2020-03-16 15:21 ` Waiman Long 2020-03-16 22:19 ` Jarkko Sakkinen 4 siblings, 2 replies; 22+ messages in thread From: David Howells @ 2020-03-16 14:24 UTC (permalink / raw) To: Waiman Long Cc: dhowells, Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen I wonder if it's worth merging this into patch 2. I'm not sure it's really worth its own patch. If you want to generalise kvzfree(), then that could go as its own patch first. David ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-16 14:24 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation David Howells @ 2020-03-16 15:21 ` Waiman Long 2020-03-16 22:19 ` Jarkko Sakkinen 1 sibling, 0 replies; 22+ messages in thread From: Waiman Long @ 2020-03-16 15:21 UTC (permalink / raw) To: David Howells Cc: Jarkko Sakkinen, James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On 3/16/20 10:24 AM, David Howells wrote: > I wonder if it's worth merging this into patch 2. I'm not sure it's really > worth its own patch. If you want to generalise kvzfree(), then that could go > as its own patch first. > > David Sure, I can merge it into patch 2. Thanks, Longman ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation 2020-03-16 14:24 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation David Howells 2020-03-16 15:21 ` Waiman Long @ 2020-03-16 22:19 ` Jarkko Sakkinen 1 sibling, 0 replies; 22+ messages in thread From: Jarkko Sakkinen @ 2020-03-16 22:19 UTC (permalink / raw) To: David Howells, Waiman Long Cc: James Morris, Serge E. Hallyn, Mimi Zohar, keyrings, linux-kernel, linux-security-module, linux-integrity, Sumit Garg, Jerry Snitselaar, Roberto Sassu, Eric Biggers, Chris von Recklinghausen On Mon, 2020-03-16 at 14:24 +0000, David Howells wrote: > I wonder if it's worth merging this into patch 2. I'm not sure it's really > worth its own patch. If you want to generalise kvzfree(), then that could go > as its own patch first. > > David Also in the sense that there is no senseful situation where you'd only wanted to pick either but not both. /Jarkko ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-03-17 18:36 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-13 15:20 [PATCH v3 0/3] KEYS: Read keys to internal buffer & then copy to userspace Waiman Long 2020-03-13 15:21 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore Waiman Long 2020-03-15 19:21 ` Jarkko Sakkinen 2020-03-15 21:27 ` Jarkko Sakkinen 2020-03-16 11:22 ` David Howells 2020-03-16 13:53 ` Jarkko Sakkinen 2020-03-16 16:33 ` Waiman Long 2020-03-17 18:10 ` Waiman Long 2020-03-16 11:34 ` David Howells 2020-03-13 15:21 ` [PATCH v3 2/3] KEYS: Avoid false positive ENOMEM error on key read Waiman Long 2020-03-15 21:32 ` Jarkko Sakkinen 2020-03-17 18:36 ` Waiman Long 2020-03-13 15:21 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation Waiman Long 2020-03-13 16:43 ` Eric Biggers 2020-03-13 17:49 ` Waiman Long 2020-03-15 21:52 ` Jarkko Sakkinen 2020-03-15 22:01 ` Waiman Long 2020-03-16 11:50 ` [PATCH v3 1/3] KEYS: Don't write out to userspace while holding key semaphore David Howells 2020-03-17 18:09 ` Waiman Long 2020-03-16 14:24 ` [PATCH v3 3/3] KEYS: Use kvmalloc() to better handle large buffer allocation David Howells 2020-03-16 15:21 ` Waiman Long 2020-03-16 22:19 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).