Re: use-after-free in irtty_open

[Dmitry Vyukov]

On Thu, Nov 26, 2015 at 3:12 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
> [ + David Miller ]
>
> On 11/25/2015 10:37 AM, Dmitry Vyukov wrote:
>> Hello,
>>
>> The following program causes a use-after-free in irtty_open:
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>> #include <sys/ioctl.h>
>>
>> int main()
>> {
>>         int fd = open("/dev/ptmx", O_RDONLY|O_NOCTTY);
>>         int val = 9;
>>         ioctl(fd, TIOCSETD, &val);
>>         val = 11;
>>         ioctl(fd, TIOCSETD, &val);
>>         return 0;
>> }
>>
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in irtty_open+0x422/0x550 at addr ffff8800331dd068
>> Read of size 4 by task a.out/13960
>> =============================================================================
>> BUG kmalloc-512 (Tainted: G    B          ): kasan: bad access detected
>> -----------------------------------------------------------------------------
>
> Thanks for the report, Dmitry.
> Would you please test the patch below?

The patch fixes the bug for me.

Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>

> As I wrote to Sasha a couple of days ago, the ldisc api should really prevent
> these kinds of errors. I have a patch for the ldisc core that eliminates
> these errors (reduces them to dead code).
>
> Seems like this was a common pattern at some point; probably need to challenge
> my coccinelle-fu to remove this pattern tree-wide.
>
> Regards,
> Peter Hurley
>
> --- >% ---
> Subject: [PATCH] net: irda: Fix use-after-free in irtty_open()
>
> The N_IRDA line discipline may access the previous line discipline's closed
> and already-fre private data on open [1].
>
> The tty->disc_data field _never_ refers to valid data on entry to the
> line discipline's open() method. Rather, the ldisc is expected to
> initialize that field for its own use for the lifetime of the instance
> (ie. from open() to close() only).
>
> [1] Report from Dmitry Vyukov <dvyukov@xxxxxxxxxx>
>     ==================================================================
>     BUG: KASAN: use-after-free in irtty_open+0x422/0x550 at addr ffff8800331dd068
>     Read of size 4 by task a.out/13960
>     =============================================================================
>     BUG kmalloc-512 (Tainted: G    B          ): kasan: bad access detected
>     -----------------------------------------------------------------------------
>     ...
>     Call Trace:
>      [<ffffffff815fa2ae>] __asan_report_load4_noabort+0x3e/0x40 mm/kasan/report.c:279
>      [<ffffffff836938a2>] irtty_open+0x422/0x550 drivers/net/irda/irtty-sir.c:436
>      [<ffffffff829f1b80>] tty_ldisc_open.isra.2+0x60/0xa0 drivers/tty/tty_ldisc.c:447
>      [<ffffffff829f21c0>] tty_set_ldisc+0x1a0/0x940 drivers/tty/tty_ldisc.c:567
>      [<     inline     >] tiocsetd drivers/tty/tty_io.c:2650
>      [<ffffffff829da49e>] tty_ioctl+0xace/0x1fd0 drivers/tty/tty_io.c:2883
>      [<     inline     >] vfs_ioctl fs/ioctl.c:43
>      [<ffffffff816708ac>] do_vfs_ioctl+0x57c/0xe60 fs/ioctl.c:607
>      [<     inline     >] SYSC_ioctl fs/ioctl.c:622
>      [<ffffffff81671204>] SyS_ioctl+0x74/0x80 fs/ioctl.c:613
>      [<ffffffff852a7876>] entry_SYSCALL_64_fastpath+0x16/0x7a
>
> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/net/irda/irtty-sir.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c
> index 696852e..7a3f990 100644
> --- a/drivers/net/irda/irtty-sir.c
> +++ b/drivers/net/irda/irtty-sir.c
> @@ -430,16 +430,6 @@ static int irtty_open(struct tty_struct *tty)
>
>         /* Module stuff handled via irda_ldisc.owner - Jean II */
>
> -       /* First make sure we're not already connected. */
> -       if (tty->disc_data != NULL) {
> -               priv = tty->disc_data;
> -               if (priv && priv->magic == IRTTY_MAGIC) {
> -                       ret = -EEXIST;
> -                       goto out;
> -               }
> -               tty->disc_data = NULL;          /* ### */
> -       }
> -
>         /* stop the underlying  driver */
>         irtty_stop_receiver(tty, TRUE);
>         if (tty->ops->stop)
> --
> 2.6.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/