* [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission
@ 2024-10-08 12:46 Chen Ridong
2024-10-16 1:33 ` Chen Ridong
0 siblings, 1 reply; 6+ messages in thread
From: Chen Ridong @ 2024-10-08 12:46 UTC (permalink / raw)
To: dhowells, jarkko, paul, jmorris, serge
Cc: keyrings, linux-security-module, linux-kernel, chenridong
From: Chen Ridong <chenridong@huawei.com>
KASAN reports an out of bounds read:
BUG: KASAN: slab-out-of-bounds in __kuid_val include/linux/uidgid.h:36
BUG: KASAN: slab-out-of-bounds in uid_eq include/linux/uidgid.h:63 [inline]
BUG: KASAN: slab-out-of-bounds in key_task_permission+0x394/0x410
security/keys/permission.c:54
Read of size 4 at addr ffff88813c3ab618 by task stress-ng/4362
CPU: 2 PID: 4362 Comm: stress-ng Not tainted 5.10.0-14930-gafbffd6c3ede #15
Call Trace:
__dump_stack lib/dump_stack.c:82 [inline]
dump_stack+0x107/0x167 lib/dump_stack.c:123
print_address_description.constprop.0+0x19/0x170 mm/kasan/report.c:400
__kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560
kasan_report+0x3a/0x50 mm/kasan/report.c:585
__kuid_val include/linux/uidgid.h:36 [inline]
uid_eq include/linux/uidgid.h:63 [inline]
key_task_permission+0x394/0x410 security/keys/permission.c:54
search_nested_keyrings+0x90e/0xe90 security/keys/keyring.c:793
This issue was also reported by syzbot [1].
It can be reproduced by following these steps(more details [2]):
1. Obtain more than 32 inputs that have similar hashes, which ends with the
pattern '0xxxxxxxe6'.
2. Reboot and add the keys obtained in step 1.
The reproducer demonstrates how this issue happened:
1. In the search_nested_keyrings function, when it iterates through the
slots in a node(below tag ascend_to_node), if the slot pointer is meta
and node->back_pointer != NULL(it means a root), it will proceed to
descend_to_node. However, there is an exception. If node is the root,
and one of the slots points to a shortcut, it will be treated as a
keyring.
2. Whether the ptr is keyring decided by keyring_ptr_is_keyring function.
However, KEYRING_PTR_SUBTYPE is 0x2UL, the same as
ASSOC_ARRAY_PTR_SUBTYPE_MASK.
3. When 32 keys with the similar hashes are added to the tree, the ROOT
has keys with hashes that are not similar (e.g. slot 0) and it splits
NODE A without using a shortcut. When NODE A is filled with keys that
all hashes are xxe6, the keys are similar, NODE A will split with a
shortcut. Finally, it forms the tree as shown below, where slot 6 points
to a shortcut.
NODE A
+------>+---+
ROOT | | 0 | xxe6
+---+ | +---+
xxxx | 0 | shortcut : : xxe6
+---+ | +---+
xxe6 : : | | | xxe6
+---+ | +---+
| 6 |---+ : : xxe6
+---+ +---+
xxe6 : : | f | xxe6
+---+ +---+
xxe6 | f |
+---+
4. As mentioned above, If a slot(slot 6) of the root points to a shortcut,
it may be mistakenly transferred to a key*, leading to a read
out-of-bounds read.
To fix this issue, one should jump to descend_to_node if the ptr is a
shortcut, regardless of whether the node is root or not.
[1] https://lore.kernel.org/all/000000000000cbb7860611f61147@google.com/T/
[2] https://lore.kernel.org/linux-kernel/1cfa878e-8c7b-4570-8606-21daf5e13ce7@huaweicloud.com/
Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
Reported-by: syzbot+5b415c07907a2990d1a3@syzkaller.appspotmail.com
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
security/keys/keyring.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 4448758f643a..f331725d5a37 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -772,8 +772,11 @@ static bool search_nested_keyrings(struct key *keyring,
for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
ptr = READ_ONCE(node->slots[slot]);
- if (assoc_array_ptr_is_meta(ptr) && node->back_pointer)
- goto descend_to_node;
+ if (assoc_array_ptr_is_meta(ptr)) {
+ if (node->back_pointer ||
+ assoc_array_ptr_is_shortcut(ptr))
+ goto descend_to_node;
+ }
if (!keyring_ptr_is_keyring(ptr))
continue;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission
2024-10-08 12:46 [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission Chen Ridong
@ 2024-10-16 1:33 ` Chen Ridong
2024-10-16 5:08 ` Jarkko Sakkinen
0 siblings, 1 reply; 6+ messages in thread
From: Chen Ridong @ 2024-10-16 1:33 UTC (permalink / raw)
To: Chen Ridong, dhowells, jarkko, paul, jmorris, serge
Cc: keyrings, linux-security-module, linux-kernel
On 2024/10/8 20:46, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> KASAN reports an out of bounds read:
> BUG: KASAN: slab-out-of-bounds in __kuid_val include/linux/uidgid.h:36
> BUG: KASAN: slab-out-of-bounds in uid_eq include/linux/uidgid.h:63 [inline]
> BUG: KASAN: slab-out-of-bounds in key_task_permission+0x394/0x410
> security/keys/permission.c:54
> Read of size 4 at addr ffff88813c3ab618 by task stress-ng/4362
>
> CPU: 2 PID: 4362 Comm: stress-ng Not tainted 5.10.0-14930-gafbffd6c3ede #15
> Call Trace:
> __dump_stack lib/dump_stack.c:82 [inline]
> dump_stack+0x107/0x167 lib/dump_stack.c:123
> print_address_description.constprop.0+0x19/0x170 mm/kasan/report.c:400
> __kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560
> kasan_report+0x3a/0x50 mm/kasan/report.c:585
> __kuid_val include/linux/uidgid.h:36 [inline]
> uid_eq include/linux/uidgid.h:63 [inline]
> key_task_permission+0x394/0x410 security/keys/permission.c:54
> search_nested_keyrings+0x90e/0xe90 security/keys/keyring.c:793
>
> This issue was also reported by syzbot [1].
>
> It can be reproduced by following these steps(more details [2]):
> 1. Obtain more than 32 inputs that have similar hashes, which ends with the
> pattern '0xxxxxxxe6'.
> 2. Reboot and add the keys obtained in step 1.
>
> The reproducer demonstrates how this issue happened:
> 1. In the search_nested_keyrings function, when it iterates through the
> slots in a node(below tag ascend_to_node), if the slot pointer is meta
> and node->back_pointer != NULL(it means a root), it will proceed to
> descend_to_node. However, there is an exception. If node is the root,
> and one of the slots points to a shortcut, it will be treated as a
> keyring.
> 2. Whether the ptr is keyring decided by keyring_ptr_is_keyring function.
> However, KEYRING_PTR_SUBTYPE is 0x2UL, the same as
> ASSOC_ARRAY_PTR_SUBTYPE_MASK.
> 3. When 32 keys with the similar hashes are added to the tree, the ROOT
> has keys with hashes that are not similar (e.g. slot 0) and it splits
> NODE A without using a shortcut. When NODE A is filled with keys that
> all hashes are xxe6, the keys are similar, NODE A will split with a
> shortcut. Finally, it forms the tree as shown below, where slot 6 points
> to a shortcut.
>
> NODE A
> +------>+---+
> ROOT | | 0 | xxe6
> +---+ | +---+
> xxxx | 0 | shortcut : : xxe6
> +---+ | +---+
> xxe6 : : | | | xxe6
> +---+ | +---+
> | 6 |---+ : : xxe6
> +---+ +---+
> xxe6 : : | f | xxe6
> +---+ +---+
> xxe6 | f |
> +---+
>
> 4. As mentioned above, If a slot(slot 6) of the root points to a shortcut,
> it may be mistakenly transferred to a key*, leading to a read
> out-of-bounds read.
>
> To fix this issue, one should jump to descend_to_node if the ptr is a
> shortcut, regardless of whether the node is root or not.
>
> [1] https://lore.kernel.org/all/000000000000cbb7860611f61147@google.com/T/
> [2] https://lore.kernel.org/linux-kernel/1cfa878e-8c7b-4570-8606-21daf5e13ce7@huaweicloud.com/
>
> Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
> Reported-by: syzbot+5b415c07907a2990d1a3@syzkaller.appspotmail.com
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
> security/keys/keyring.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 4448758f643a..f331725d5a37 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -772,8 +772,11 @@ static bool search_nested_keyrings(struct key *keyring,
> for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
> ptr = READ_ONCE(node->slots[slot]);
>
> - if (assoc_array_ptr_is_meta(ptr) && node->back_pointer)
> - goto descend_to_node;
> + if (assoc_array_ptr_is_meta(ptr)) {
> + if (node->back_pointer ||
> + assoc_array_ptr_is_shortcut(ptr))
> + goto descend_to_node;
> + }
>
> if (!keyring_ptr_is_keyring(ptr))
> continue;
Friendly ping.
Best regards,
Ridong
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission
2024-10-16 1:33 ` Chen Ridong
@ 2024-10-16 5:08 ` Jarkko Sakkinen
2024-10-16 5:14 ` Jarkko Sakkinen
0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2024-10-16 5:08 UTC (permalink / raw)
To: Chen Ridong, dhowells, paul, jmorris, serge
Cc: keyrings, linux-security-module, linux-kernel
On Wed, 2024-10-16 at 09:33 +0800, Chen Ridong wrote:
>
>
> On 2024/10/8 20:46, Chen Ridong wrote:
> > From: Chen Ridong <chenridong@huawei.com>
> >
> > KASAN reports an out of bounds read:
> > BUG: KASAN: slab-out-of-bounds in __kuid_val
> > include/linux/uidgid.h:36
> > BUG: KASAN: slab-out-of-bounds in uid_eq include/linux/uidgid.h:63
> > [inline]
> > BUG: KASAN: slab-out-of-bounds in key_task_permission+0x394/0x410
> > security/keys/permission.c:54
> > Read of size 4 at addr ffff88813c3ab618 by task stress-ng/4362
> >
> > CPU: 2 PID: 4362 Comm: stress-ng Not tainted 5.10.0-14930-
> > gafbffd6c3ede #15
> > Call Trace:
> > __dump_stack lib/dump_stack.c:82 [inline]
> > dump_stack+0x107/0x167 lib/dump_stack.c:123
> > print_address_description.constprop.0+0x19/0x170
> > mm/kasan/report.c:400
> > __kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560
> > kasan_report+0x3a/0x50 mm/kasan/report.c:585
> > __kuid_val include/linux/uidgid.h:36 [inline]
> > uid_eq include/linux/uidgid.h:63 [inline]
> > key_task_permission+0x394/0x410 security/keys/permission.c:54
> > search_nested_keyrings+0x90e/0xe90 security/keys/keyring.c:793
> >
> > This issue was also reported by syzbot [1].
> >
> > It can be reproduced by following these steps(more details [2]):
> > 1. Obtain more than 32 inputs that have similar hashes, which ends
> > with the
> > pattern '0xxxxxxxe6'.
> > 2. Reboot and add the keys obtained in step 1.
> >
> > The reproducer demonstrates how this issue happened:
> > 1. In the search_nested_keyrings function, when it iterates through
> > the
> > slots in a node(below tag ascend_to_node), if the slot pointer
> > is meta
> > and node->back_pointer != NULL(it means a root), it will
> > proceed to
> > descend_to_node. However, there is an exception. If node is the
> > root,
> > and one of the slots points to a shortcut, it will be treated
> > as a
> > keyring.
> > 2. Whether the ptr is keyring decided by keyring_ptr_is_keyring
> > function.
> > However, KEYRING_PTR_SUBTYPE is 0x2UL, the same as
> > ASSOC_ARRAY_PTR_SUBTYPE_MASK.
> > 3. When 32 keys with the similar hashes are added to the tree, the
> > ROOT
> > has keys with hashes that are not similar (e.g. slot 0) and it
> > splits
> > NODE A without using a shortcut. When NODE A is filled with
> > keys that
> > all hashes are xxe6, the keys are similar, NODE A will split
> > with a
> > shortcut. Finally, it forms the tree as shown below, where slot
> > 6 points
> > to a shortcut.
> >
> > NODE A
> > +------>+---+
> > ROOT | | 0 | xxe6
> > +---+ | +---+
> > xxxx | 0 | shortcut : : xxe6
> > +---+ | +---+
> > xxe6 : : | | | xxe6
> > +---+ | +---+
> > | 6 |---+ : : xxe6
> > +---+ +---+
> > xxe6 : : | f | xxe6
> > +---+ +---+
> > xxe6 | f |
> > +---+
> >
> > 4. As mentioned above, If a slot(slot 6) of the root points to a
> > shortcut,
> > it may be mistakenly transferred to a key*, leading to a read
> > out-of-bounds read.
> >
> > To fix this issue, one should jump to descend_to_node if the ptr is
> > a
> > shortcut, regardless of whether the node is root or not.
> >
> > [1]
> > https://lore.kernel.org/all/000000000000cbb7860611f61147@google.com/T/
> > [2]
> > https://lore.kernel.org/linux-kernel/1cfa878e-8c7b-4570-8606-21daf5e13ce7@huaweicloud.com/
> >
> > Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
> > Reported-by: syzbot+5b415c07907a2990d1a3@syzkaller.appspotmail.com
> > Signed-off-by: Chen Ridong <chenridong@huawei.com>
> > ---
> > security/keys/keyring.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> > index 4448758f643a..f331725d5a37 100644
> > --- a/security/keys/keyring.c
> > +++ b/security/keys/keyring.c
> > @@ -772,8 +772,11 @@ static bool search_nested_keyrings(struct key
> > *keyring,
> > for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
> > ptr = READ_ONCE(node->slots[slot]);
> >
> > - if (assoc_array_ptr_is_meta(ptr) && node-
> > >back_pointer)
> > - goto descend_to_node;
> > + if (assoc_array_ptr_is_meta(ptr)) {
> > + if (node->back_pointer ||
> > + assoc_array_ptr_is_shortcut(ptr))
> > + goto descend_to_node;
> > + }
> >
> > if (!keyring_ptr_is_keyring(ptr))
> > continue;
>
> Friendly ping.
Thanks for pinging because I actually accidentally missed the original
email!
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
I'll pick this up.
>
> Best regards,
> Ridong
>
>
BR, Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission
2024-10-16 5:08 ` Jarkko Sakkinen
@ 2024-10-16 5:14 ` Jarkko Sakkinen
2024-10-17 1:19 ` Chen Ridong
0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2024-10-16 5:14 UTC (permalink / raw)
To: Chen Ridong, dhowells, paul, jmorris, serge
Cc: keyrings, linux-security-module, linux-kernel
On Wed, 2024-10-16 at 08:08 +0300, Jarkko Sakkinen wrote:
> > Friendly ping.
>
> Thanks for pinging because I actually accidentally missed the
> original
> email!
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> I'll pick this up.
I tuned the commit message just a bit (see my comment
embedded):
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=9bb3ba75c1c8fd8c9f6a0b1fd6409b725583a3e0
BR, Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission
2024-10-16 5:14 ` Jarkko Sakkinen
@ 2024-10-17 1:19 ` Chen Ridong
2024-10-17 8:39 ` Jarkko Sakkinen
0 siblings, 1 reply; 6+ messages in thread
From: Chen Ridong @ 2024-10-17 1:19 UTC (permalink / raw)
To: Jarkko Sakkinen, dhowells, paul, jmorris, serge
Cc: keyrings, linux-security-module, linux-kernel
On 2024/10/16 13:14, Jarkko Sakkinen wrote:
> On Wed, 2024-10-16 at 08:08 +0300, Jarkko Sakkinen wrote:
>>> Friendly ping.
>>
>> Thanks for pinging because I actually accidentally missed the
>> original
>> email!
>>
>> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>>
>> I'll pick this up.
>
> I tuned the commit message just a bit (see my comment
> embedded):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=9bb3ba75c1c8fd8c9f6a0b1fd6409b725583a3e0
>
> BR, Jarkko
Thanks you very much.
Best regards,
Ridong
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission
2024-10-17 1:19 ` Chen Ridong
@ 2024-10-17 8:39 ` Jarkko Sakkinen
0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2024-10-17 8:39 UTC (permalink / raw)
To: Chen Ridong, dhowells, paul, jmorris, serge
Cc: keyrings, linux-security-module, linux-kernel
On Thu, 2024-10-17 at 09:19 +0800, Chen Ridong wrote:
>
>
> On 2024/10/16 13:14, Jarkko Sakkinen wrote:
> > On Wed, 2024-10-16 at 08:08 +0300, Jarkko Sakkinen wrote:
> > > > Friendly ping.
> > >
> > > Thanks for pinging because I actually accidentally missed the
> > > original
> > > email!
> > >
> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > >
> > > I'll pick this up.
> >
> > I tuned the commit message just a bit (see my comment
> > embedded):
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=9bb3ba75c1c8fd8c9f6a0b1fd6409b725583a3e0
> >
> > BR, Jarkko
>
> Thanks you very much.
Thanks to you for patience with my earlier shitty reviews ;-) That was
tbh the main issue with this bug fix getting through...
> Best regards,
> Ridong
BR, Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-17 8:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-08 12:46 [PATCH v2] security/keys: fix slab-out-of-bounds in key_task_permission Chen Ridong
2024-10-16 1:33 ` Chen Ridong
2024-10-16 5:08 ` Jarkko Sakkinen
2024-10-16 5:14 ` Jarkko Sakkinen
2024-10-17 1:19 ` Chen Ridong
2024-10-17 8:39 ` 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).