linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL
       [not found] ` <20190119091108.GF10836@kroah.com>
@ 2019-01-20  9:52   ` Greg Kroah-Hartman
  2019-01-21 15:38     ` Jann Horn
  0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-20  9:52 UTC (permalink / raw)
  To: Jann Horn; +Cc: Jiri Slaby, kernel list, linux-serial

On Sat, Jan 19, 2019 at 10:11:08AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Jan 18, 2019 at 08:09:07PM +0100, Jann Horn wrote:
> > Hi!
> > 
> > When a line discipline doesn't have a ->receive_buf handler, tiocsti()
> > attempts to call a NULL pointer. Both tty_n_tracesink and
> > spk_ttyio_ldisc_ops don't have such a handler.
> > 
> > To reproduce, build a kernel with CONFIG_SPEAKUP=y and
> > CONFIG_SPEAKUP_SYNTH_SOFT=y, set speakup.synth=soft in the kernel
> > command line, and run the following code as root:
> 
> <snip>
> 
> Ugh, thanks for finding this.  I'll look at it later this afternoon...

It looks to be a simple change.  We can't really "fail" this ioctl if
there's nothing wrong with the structure of the call, so we can just
quietly "eat" the character, given that the line discipline doesn't care
about it.

So, any objections to the patch below?

thanks,

greg k-h

-----------------

Subject: [PATCH] tty: Handle problem if line discipline does not have receive_buf

Some tty line disciplines do not have a receive buf callback, so
properly check for that before calling it.  If they do not have this
callback, just eat the character quietly, as we can't fail this call.

Reported-by: Jann Horn <jannh@google.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/tty_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 23c6fd238422..21ffcce16927 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2189,7 +2189,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
 	ld = tty_ldisc_ref_wait(tty);
 	if (!ld)
 		return -EIO;
-	ld->ops->receive_buf(tty, &ch, &mbz, 1);
+	if (ld->ops->receive_buf)
+		ld->ops->receive_buf(tty, &ch, &mbz, 1);
 	tty_ldisc_deref(ld);
 	return 0;
 }
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL
  2019-01-20  9:52   ` [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL Greg Kroah-Hartman
@ 2019-01-21 15:38     ` Jann Horn
  2019-01-21 16:14       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Jann Horn @ 2019-01-21 15:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, kernel list, linux-serial

On Sun, Jan 20, 2019 at 10:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jan 19, 2019 at 10:11:08AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Jan 18, 2019 at 08:09:07PM +0100, Jann Horn wrote:
> > > Hi!
> > >
> > > When a line discipline doesn't have a ->receive_buf handler, tiocsti()
> > > attempts to call a NULL pointer. Both tty_n_tracesink and
> > > spk_ttyio_ldisc_ops don't have such a handler.
> > >
> > > To reproduce, build a kernel with CONFIG_SPEAKUP=y and
> > > CONFIG_SPEAKUP_SYNTH_SOFT=y, set speakup.synth=soft in the kernel
> > > command line, and run the following code as root:
> >
> > <snip>
> >
> > Ugh, thanks for finding this.  I'll look at it later this afternoon...
>
> It looks to be a simple change.  We can't really "fail" this ioctl if
> there's nothing wrong with the structure of the call, so we can just
> quietly "eat" the character, given that the line discipline doesn't care
> about it.
>
> So, any objections to the patch below?

No objection from me.

(spk_ttyio_ldisc_ops has a receive_buf2 handler, but I don't know
whether that should be invoked here or not.)

> -----------------
>
> Subject: [PATCH] tty: Handle problem if line discipline does not have receive_buf
>
> Some tty line disciplines do not have a receive buf callback, so
> properly check for that before calling it.  If they do not have this
> callback, just eat the character quietly, as we can't fail this call.
>
> Reported-by: Jann Horn <jannh@google.com>
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/tty_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 23c6fd238422..21ffcce16927 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2189,7 +2189,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
>         ld = tty_ldisc_ref_wait(tty);
>         if (!ld)
>                 return -EIO;
> -       ld->ops->receive_buf(tty, &ch, &mbz, 1);
> +       if (ld->ops->receive_buf)
> +               ld->ops->receive_buf(tty, &ch, &mbz, 1);
>         tty_ldisc_deref(ld);
>         return 0;
>  }
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL
  2019-01-21 15:38     ` Jann Horn
@ 2019-01-21 16:14       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-21 16:14 UTC (permalink / raw)
  To: Jann Horn; +Cc: Jiri Slaby, kernel list, linux-serial

On Mon, Jan 21, 2019 at 04:38:33PM +0100, Jann Horn wrote:
> On Sun, Jan 20, 2019 at 10:52 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Jan 19, 2019 at 10:11:08AM +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Jan 18, 2019 at 08:09:07PM +0100, Jann Horn wrote:
> > > > Hi!
> > > >
> > > > When a line discipline doesn't have a ->receive_buf handler, tiocsti()
> > > > attempts to call a NULL pointer. Both tty_n_tracesink and
> > > > spk_ttyio_ldisc_ops don't have such a handler.
> > > >
> > > > To reproduce, build a kernel with CONFIG_SPEAKUP=y and
> > > > CONFIG_SPEAKUP_SYNTH_SOFT=y, set speakup.synth=soft in the kernel
> > > > command line, and run the following code as root:
> > >
> > > <snip>
> > >
> > > Ugh, thanks for finding this.  I'll look at it later this afternoon...
> >
> > It looks to be a simple change.  We can't really "fail" this ioctl if
> > there's nothing wrong with the structure of the call, so we can just
> > quietly "eat" the character, given that the line discipline doesn't care
> > about it.
> >
> > So, any objections to the patch below?
> 
> No objection from me.
> 
> (spk_ttyio_ldisc_ops has a receive_buf2 handler, but I don't know
> whether that should be invoked here or not.)

No, receive_buf2 can fail, or do a short "receive", which receive_buf()
can't do, and tiocsti can't fail (it's only used to fake input data).

Yeah, the tty layer is strange :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-01-21 16:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAG48ez06mDExELyTYN7MZQ3FjGcLx9ASc5HnxW8NQbfTAPX=Qg@mail.gmail.com>
     [not found] ` <20190119091108.GF10836@kroah.com>
2019-01-20  9:52   ` [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL Greg Kroah-Hartman
2019-01-21 15:38     ` Jann Horn
2019-01-21 16:14       ` Greg Kroah-Hartman

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).