linux-kernel - Re: [PATCH v4] tty: serial: jsm: allocate queue buffer at probe time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMhUBj=Vrwd__fmTmegqU22Hn3zGE_iitF0+zAAkQFHssy3gaA@mail.gmail.com>
Date:   Wed, 7 Jul 2021 20:28:12 +0800
From:   Zheyu Ma <zheyuma97@...il.com>
To:     Jiri Slaby <jirislaby@...nel.org>
Cc:     Greg KH <gregkh@...uxfoundation.org>, linux-serial@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] tty: serial: jsm: allocate queue buffer at probe time

On Wed, Jul 7, 2021 at 3:49 PM Jiri Slaby <jirislaby@...nel.org> wrote:
>
> On 05. 07. 21, 14:53, Zheyu Ma wrote:
> > In function 'neo_intr', the driver uses 'ch->ch_equeue' and
> > 'ch->ch_reuque'. These two pointers are initialized in 'jsm_tty_open',
> > but the interrupt handler 'neo_intr' has been registered in the probe
> > progress. If 'jsm_tty_open' has not been called at this time, it will
> > cause null pointer dereference.
> >
> > Once the driver registers the interrupt handler, the driver should be
> > ready to handle it.
> >
> > Fix this by allocating the memory at probe time and not at open time.
>
> You are allocating the buffer in jsm_tty_init now. But that is still
> called after request_irq() in probe. So care to explain how this helps
> exactly? As I understand it, you only made the window much smaller.

You are right, this may indeed still cause problems, I did not
consider this before. So maybe we should put request_irq() at the end
of the probe function.

> Anyway, I'm no expert on jsm, but AFAICS jsm_tty_open first allocates
> the buffers, brd->bd_ops->uart_init() / neo_uart_init() clears ier and
> only brd->bd_ops->param() / neo_param() enables interrupts on the device
> (by ier update and write). So how it comes an interrupt came before
> neo_param() in jsm_tty_open was called?

I considered the threat from a malicious device, which means that a
harmful peripheral may not comply with the driver's convention,
arbitrary send interrupt signals, or send malicious data. I think the
driver should also handle this situation, at least to prevent the
kernel from crashing.

Thanks,

Zheyu Ma