2023-12-12 11:18:06

by Gavrilov Ilia

[permalink] [raw]
Subject: [PATCH 5.10 0/3] tty: n_gsm: fix tty registration before control channel open

Syzkaller reports memory leak issue at gsmld_attach_gsm() in
5.10 stable releases. The reproducer injects the memory allocation
errors to tty_register_device(); as a result, tty_kref_get() isn't called
after this error, which leads to tty_struct leak.
The issue has been fixed by the following patches that can be cleanly
applied to the 5.10 branch.

Found by InfoTeCS on behalf of Linux Verification Center
(linuxtesting.org) with Syzkaller


2023-12-12 11:18:14

by Gavrilov Ilia

[permalink] [raw]
Subject: [PATCH 5.10 2/3] tty: n_gsm, remove duplicates of parameters

From: Jiri Slaby <[email protected]>

commit b93db97e1ca08e500305bc46b08c72e2232c4be1 upstream.

dp, f, and i are only duplicates of gsmld_receive_buf's parameters. Use
the parameters directly (cp, fp, and count) and delete these local
variables.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Gavrilov Ilia <[email protected]>
---
drivers/tty/n_gsm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 7a883a2c0c50..2455f952e0aa 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2577,27 +2577,24 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
char *fp, int count)
{
struct gsm_mux *gsm = tty->disc_data;
- const unsigned char *dp;
- char *f;
- int i;
char flags = TTY_NORMAL;

if (debug & 4)
print_hex_dump_bytes("gsmld_receive: ", DUMP_PREFIX_OFFSET,
cp, count);

- for (i = count, dp = cp, f = fp; i; i--, dp++) {
- if (f)
- flags = *f++;
+ for (; count; count--, cp++) {
+ if (fp)
+ flags = *fp++;
switch (flags) {
case TTY_NORMAL:
- gsm->receive(gsm, *dp);
+ gsm->receive(gsm, *cp);
break;
case TTY_OVERRUN:
case TTY_BREAK:
case TTY_PARITY:
case TTY_FRAME:
- gsm_error(gsm, *dp, flags);
+ gsm_error(gsm, *cp, flags);
break;
default:
WARN_ONCE(1, "%s: unknown flag %d\n",
--
2.39.2

2023-12-12 11:44:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5.10 0/3] tty: n_gsm: fix tty registration before control channel open

On Tue, Dec 12, 2023 at 11:17:21AM +0000, Gavrilov Ilia wrote:
> Syzkaller reports memory leak issue at gsmld_attach_gsm() in
> 5.10 stable releases. The reproducer injects the memory allocation
> errors to tty_register_device(); as a result, tty_kref_get() isn't called
> after this error, which leads to tty_struct leak.
> The issue has been fixed by the following patches that can be cleanly
> applied to the 5.10 branch.
>
> Found by InfoTeCS on behalf of Linux Verification Center
> (linuxtesting.org) with Syzkaller

Do you actually have any hardware for this protocol running on the
5.10.y kernel? How was this tested? Why was just this specific set of
patches picked to be backported?

thanks,

greg k-h

2023-12-12 12:26:43

by Gavrilov Ilia

[permalink] [raw]
Subject: Re: [PATCH 5.10 0/3] tty: n_gsm: fix tty registration before control channel open

On 12/12/23 14:44, Greg Kroah-Hartman wrote:
> On Tue, Dec 12, 2023 at 11:17:21AM +0000, Gavrilov Ilia wrote:
>> Syzkaller reports memory leak issue at gsmld_attach_gsm() in
>> 5.10 stable releases. The reproducer injects the memory allocation
>> errors to tty_register_device(); as a result, tty_kref_get() isn't called
>> after this error, which leads to tty_struct leak.
>> The issue has been fixed by the following patches that can be cleanly
>> applied to the 5.10 branch.
>>
>> Found by InfoTeCS on behalf of Linux Verification Center
>> (linuxtesting.org) with Syzkaller
>
> Do you actually have any hardware for this protocol running on the
> 5.10.y kernel? How was this tested? Why was just this specific set of
> patches picked to be backported?
>

No, I don't have any hardware for this protocol. I tested this manually
on virtual machines and using a reproducer (generated by syzkaller).
The first patch fixes the main problem(memory leak). The third patch
fixes the problem with а null pointer dereference. I added this patch
because it has a "fixes" tag that references to the first patch. The
third patch can't be applied cleanly without the second patch.

> thanks,
>
> greg k-h