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

[Zheyu Ma <zheyuma97@gmail.com>]

On Wed, Jul 7, 2021 at 3:49 PM Jiri Slaby <jirislaby@kernel.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