linux-kernel - Re: [PATCH] kconfig: Add kernel config option for fuzz testing.

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]

Date:   Tue, 17 Dec 2019 21:54:02 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kconfig: Add kernel config option for fuzz testing.

On 2019/12/17 17:41, Dmitry Vyukov wrote:
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 90655910b0c7..367b92ad598b 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -519,11 +519,14 @@ serial_port_out_sync(struct uart_port *p, int offset, int value)
>>         case UPIO_MEM32:
>>         case UPIO_MEM32BE:
>>         case UPIO_AU:
>> -               p->serial_out(p, offset, value);
>> +               /* Writing to random kernel address causes crash. */
>> +               if (!IS_ENABLED(CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING))
>> +                       p->serial_out(p, offset, value);
> 
> Does this do the same as LOCKDOWN_TIOCSSERIAL? How is it different?

I don't know. If there were an oversight in condition of lines 852-858,
uart_startup() might be called due to "goto check_and_exit;" without
hitting security_locked_down(LOCKDOWN_TIOCSSERIAL) check.

846:    old_flags = uport->flags;
847:    new_flags = (__force upf_t)new_info->flags;
848:    old_custom_divisor = uport->custom_divisor;
849:
850:    if (!capable(CAP_SYS_ADMIN)) {
851:            retval = -EPERM;
852:            if (change_irq || change_port ||
853:                (new_info->baud_base != uport->uartclk / 16) ||
854:                (close_delay != port->close_delay) ||
855:                (closing_wait != port->closing_wait) ||
856:                (new_info->xmit_fifo_size &&
857:                 new_info->xmit_fifo_size != uport->fifosize) ||
858:                (((new_flags ^ old_flags) & ~UPF_USR_MASK) != 0))
859:                    goto exit;
860:            uport->flags = ((uport->flags & ~UPF_USR_MASK) |
861:                           (new_flags & UPF_USR_MASK));
862:            uport->custom_divisor = new_info->custom_divisor;
863:            goto check_and_exit;
864:    }
865:
866:    retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
867:    if (retval && (change_irq || change_port))
868:            goto exit;

> 
>>                 p->serial_in(p, UART_LCR);      /* safe, no side-effects */
>>                 break;
>>         default:
>> -               p->serial_out(p, offset, value);
>> +               if (!IS_ENABLED(CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING))
>> +                       p->serial_out(p, offset, value);
>>         }
>>  }

But I came think that "BUG: unable to handle kernel NULL pointer dereference in
mem_serial_out" is a real kernel bug which should be fixed. It seems that crash
occurs only when "struct serial_struct"->iomem_base == NULL, and EBUSY is
returned otherwise. That is, some sanity check is wrong.