2019-12-19 12:15:15

by Daniel Axtens

[permalink] [raw]
Subject: [PATCH v2] relay: handle alloc_percpu returning NULL in relay_open

alloc_percpu() may return NULL, which means chan->buf may be set to
NULL. In that case, when we do *per_cpu_ptr(chan->buf, ...), we
dereference an invalid pointer:

BUG: Unable to handle kernel data access at 0x7dae0000
Faulting instruction address: 0xc0000000003f3fec
...
NIP [c0000000003f3fec] relay_open+0x29c/0x600
LR [c0000000003f3fc0] relay_open+0x270/0x600
Call Trace:
[c000000054353a70] [c0000000003f3fb4] relay_open+0x264/0x600 (unreliable)
[c000000054353b00] [c000000000451764] __blk_trace_setup+0x254/0x600
[c000000054353bb0] [c000000000451b78] blk_trace_setup+0x68/0xa0
[c000000054353c10] [c0000000010da77c] sg_ioctl+0x7bc/0x2e80
[c000000054353cd0] [c000000000758cbc] do_vfs_ioctl+0x13c/0x1300
[c000000054353d90] [c000000000759f14] ksys_ioctl+0x94/0x130
[c000000054353de0] [c000000000759ff8] sys_ioctl+0x48/0xb0
[c000000054353e20] [c00000000000bcd0] system_call+0x5c/0x68

Check if alloc_percpu returns NULL.

This was found by syzkaller both on x86 and powerpc, and the reproducer
it found on powerpc is capable of hitting the issue as an unprivileged
user.

Fixes: 017c59c042d0 ("relay: Use per CPU constructs for the relay channel buffer pointers")
Reported-by: [email protected]
Reported-by: [email protected]
Reported-by: [email protected]
Reported-by: [email protected]
Cc: Akash Goel <[email protected]>
Cc: Andrew Donnellan <[email protected]> # syzkaller-ppc64
Reviewed-by: Michael Ellerman <[email protected]>
Reviewed-by: Andrew Donnellan <[email protected]>
Cc: [email protected] # v4.10+
Signed-off-by: Daniel Axtens <[email protected]>

--

v2: drop the NOWARN.

There's a syz reproducer on the powerpc syzbot that eventually hits
the bug, but it can take up to an hour or so before it keels over on a
kernel with all the syzkaller debugging on, and even longer on a
production kernel. I have been able to reproduce it once on a stock
Ubuntu 5.0 ppc64le kernel.

CVE-2019-19462 has been assigned. While only the process doing the syscall
gets killed, it gets killed while holding the relay_channels_mutex,
so it blocks all future relay activity.
---
kernel/relay.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/relay.c b/kernel/relay.c
index ade14fb7ce2e..4b760ec16342 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -581,6 +581,11 @@ struct rchan *relay_open(const char *base_filename,
return NULL;

chan->buf = alloc_percpu(struct rchan_buf *);
+ if (!chan->buf) {
+ kfree(chan);
+ return NULL;
+ }
+
chan->version = RELAYFS_CHANNEL_VERSION;
chan->n_subbufs = n_subbufs;
chan->subbuf_size = subbuf_size;
--
2.20.1


2019-12-20 19:02:47

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH v2] relay: handle alloc_percpu returning NULL in relay_open

On Thu, 19 Dec 2019, Daniel Axtens wrote:

> alloc_percpu() may return NULL, which means chan->buf may be set to
> NULL. In that case, when we do *per_cpu_ptr(chan->buf, ...), we
> dereference an invalid pointer:
>
> BUG: Unable to handle kernel data access at 0x7dae0000
> Faulting instruction address: 0xc0000000003f3fec
> ...
> NIP [c0000000003f3fec] relay_open+0x29c/0x600
> LR [c0000000003f3fc0] relay_open+0x270/0x600
> Call Trace:
> [c000000054353a70] [c0000000003f3fb4] relay_open+0x264/0x600 (unreliable)
> [c000000054353b00] [c000000000451764] __blk_trace_setup+0x254/0x600
> [c000000054353bb0] [c000000000451b78] blk_trace_setup+0x68/0xa0
> [c000000054353c10] [c0000000010da77c] sg_ioctl+0x7bc/0x2e80
> [c000000054353cd0] [c000000000758cbc] do_vfs_ioctl+0x13c/0x1300
> [c000000054353d90] [c000000000759f14] ksys_ioctl+0x94/0x130
> [c000000054353de0] [c000000000759ff8] sys_ioctl+0x48/0xb0
> [c000000054353e20] [c00000000000bcd0] system_call+0x5c/0x68
>
> Check if alloc_percpu returns NULL.
>
> This was found by syzkaller both on x86 and powerpc, and the reproducer
> it found on powerpc is capable of hitting the issue as an unprivileged
> user.
>
> Fixes: 017c59c042d0 ("relay: Use per CPU constructs for the relay channel buffer pointers")
> Reported-by: [email protected]
> Reported-by: [email protected]
> Reported-by: [email protected]
> Reported-by: [email protected]
> Cc: Akash Goel <[email protected]>
> Cc: Andrew Donnellan <[email protected]> # syzkaller-ppc64
> Reviewed-by: Michael Ellerman <[email protected]>
> Reviewed-by: Andrew Donnellan <[email protected]>
> Cc: [email protected] # v4.10+
> Signed-off-by: Daniel Axtens <[email protected]>

Acked-by: David Rientjes <[email protected]>

2020-05-21 15:27:47

by Salvatore Bonaccorso

[permalink] [raw]
Subject: Re: [PATCH v2] relay: handle alloc_percpu returning NULL in relay_open

Hi,

On Fri, Dec 20, 2019 at 11:00:56AM -0800, David Rientjes wrote:
> On Thu, 19 Dec 2019, Daniel Axtens wrote:
>
> > alloc_percpu() may return NULL, which means chan->buf may be set to
> > NULL. In that case, when we do *per_cpu_ptr(chan->buf, ...), we
> > dereference an invalid pointer:
> >
> > BUG: Unable to handle kernel data access at 0x7dae0000
> > Faulting instruction address: 0xc0000000003f3fec
> > ...
> > NIP [c0000000003f3fec] relay_open+0x29c/0x600
> > LR [c0000000003f3fc0] relay_open+0x270/0x600
> > Call Trace:
> > [c000000054353a70] [c0000000003f3fb4] relay_open+0x264/0x600 (unreliable)
> > [c000000054353b00] [c000000000451764] __blk_trace_setup+0x254/0x600
> > [c000000054353bb0] [c000000000451b78] blk_trace_setup+0x68/0xa0
> > [c000000054353c10] [c0000000010da77c] sg_ioctl+0x7bc/0x2e80
> > [c000000054353cd0] [c000000000758cbc] do_vfs_ioctl+0x13c/0x1300
> > [c000000054353d90] [c000000000759f14] ksys_ioctl+0x94/0x130
> > [c000000054353de0] [c000000000759ff8] sys_ioctl+0x48/0xb0
> > [c000000054353e20] [c00000000000bcd0] system_call+0x5c/0x68
> >
> > Check if alloc_percpu returns NULL.
> >
> > This was found by syzkaller both on x86 and powerpc, and the reproducer
> > it found on powerpc is capable of hitting the issue as an unprivileged
> > user.
> >
> > Fixes: 017c59c042d0 ("relay: Use per CPU constructs for the relay channel buffer pointers")
> > Reported-by: [email protected]
> > Reported-by: [email protected]
> > Reported-by: [email protected]
> > Reported-by: [email protected]
> > Cc: Akash Goel <[email protected]>
> > Cc: Andrew Donnellan <[email protected]> # syzkaller-ppc64
> > Reviewed-by: Michael Ellerman <[email protected]>
> > Reviewed-by: Andrew Donnellan <[email protected]>
> > Cc: [email protected] # v4.10+
> > Signed-off-by: Daniel Axtens <[email protected]>
>
> Acked-by: David Rientjes <[email protected]>

It looks this one was never applied (which relates to CVE-2019-19462,
as pointed by Guenter in [email protected]).

Whas this lost or are there any issues pending?

Regards,
Salvatore

2020-05-25 01:57:48

by Daniel Axtens

[permalink] [raw]
Subject: Re: [PATCH v2] relay: handle alloc_percpu returning NULL in relay_open

>> > Check if alloc_percpu returns NULL.
>> >
>> > This was found by syzkaller both on x86 and powerpc, and the reproducer
>> > it found on powerpc is capable of hitting the issue as an unprivileged
>> > user.
>> >
>> > Fixes: 017c59c042d0 ("relay: Use per CPU constructs for the relay channel buffer pointers")
>> > Reported-by: [email protected]
>> > Reported-by: [email protected]
>> > Reported-by: [email protected]
>> > Reported-by: [email protected]
>> > Cc: Akash Goel <[email protected]>
>> > Cc: Andrew Donnellan <[email protected]> # syzkaller-ppc64
>> > Reviewed-by: Michael Ellerman <[email protected]>
>> > Reviewed-by: Andrew Donnellan <[email protected]>
>> > Cc: [email protected] # v4.10+
>> > Signed-off-by: Daniel Axtens <[email protected]>
>>
>> Acked-by: David Rientjes <[email protected]>
>
> It looks this one was never applied (which relates to CVE-2019-19462,
> as pointed by Guenter in [email protected]).
>
> Whas this lost or are there any issues pending?

I'm not aware of any pending issues.

(But, if anyone does have any objections I'm happy to revise the patch.)

Regards,
Daniel

2020-05-26 03:27:20

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2] relay: handle alloc_percpu returning NULL in relay_open

[ + akpm ]

Daniel Axtens <[email protected]> writes:
>>> > Check if alloc_percpu returns NULL.
>>> >
>>> > This was found by syzkaller both on x86 and powerpc, and the reproducer
>>> > it found on powerpc is capable of hitting the issue as an unprivileged
>>> > user.
>>> >
>>> > Fixes: 017c59c042d0 ("relay: Use per CPU constructs for the relay channel buffer pointers")
>>> > Reported-by: [email protected]
>>> > Reported-by: [email protected]
>>> > Reported-by: [email protected]
>>> > Reported-by: [email protected]
>>> > Cc: Akash Goel <[email protected]>
>>> > Cc: Andrew Donnellan <[email protected]> # syzkaller-ppc64
>>> > Reviewed-by: Michael Ellerman <[email protected]>
>>> > Reviewed-by: Andrew Donnellan <[email protected]>
>>> > Cc: [email protected] # v4.10+
>>> > Signed-off-by: Daniel Axtens <[email protected]>
>>>
>>> Acked-by: David Rientjes <[email protected]>
>>
>> It looks this one was never applied (which relates to CVE-2019-19462,
>> as pointed by Guenter in [email protected]).
>>
>> Whas this lost or are there any issues pending?
>
> I'm not aware of any pending issues.
>
> (But, if anyone does have any objections I'm happy to revise the patch.)

It looks like kernel/relay.c is lacking a maintainer?

Andrew are you able to pick this up for v5.8? It's pretty obviously
correct, and has David's ack.

Original is here if that helps:
https://lore.kernel.org/lkml/[email protected]/


cheers