From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52362)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1as68X-0008Qw-0P
	for qemu-devel@nongnu.org; Mon, 18 Apr 2016 06:10:34 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1as68Q-0002iW-Vk
	for qemu-devel@nongnu.org; Mon, 18 Apr 2016 06:10:32 -0400
Received: from mail-vk0-x229.google.com ([2607:f8b0:400c:c05::229]:35347)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1as68Q-0002iO-AM
	for qemu-devel@nongnu.org; Mon, 18 Apr 2016 06:10:26 -0400
Received: by mail-vk0-x229.google.com with SMTP id t129so214366671vkg.2
	for <qemu-devel@nongnu.org>; Mon, 18 Apr 2016 03:10:26 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <20160418100735.GA517@redhat.com>
References: <143C0AFC63FC204CB0C55BB88F3A8ABBE31B34@EX01.corp.qihoo.net>
	<20160418100735.GA517@redhat.com>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Mon, 18 Apr 2016 11:10:06 +0100
Message-ID: <CAFEAcA8mrU9HW6C_T0Ba3Ls6xH2TstS6WbQW0utx11kFocGq2Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH] cadence_uart: bounds check write offset
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>, =?UTF-8?B?5p2O5by6?= <liqiang6-s@360.cn>, "pmatouse@redhat.com" <pmatouse@redhat.com>, "sstabellini@kernel.org" <sstabellini@kernel.org>, "secalert@redhat.com" <secalert@redhat.com>, "mdroth@linux.vnet.ibm.com" <mdroth@linux.vnet.ibm.com>, g-sec-cloud <g-sec-cloud@list.qihoo.net>, Alistair Francis <alistair.francis@xilinx.com>, Peter Crosthwaite <crosthwaite.peter@gmail.com>

CCing the maintainers for this device...

On 18 April 2016 at 11:07, Michael S. Tsirkin <mst@redhat.com> wrote:
> cadence_uart_init() initializes an I/O memory region of size 0x1000
> bytes.  However in uart_write(), the 'offset' parameter (offset within
> region) is divided by 4 and then used to index the array 'r' of size
> CADENCE_UART_R_MAX which is much smaller: (0x48/4).  If 'offset>>=3D2'
> exceeds CADENCE_UART_R_MAX, this will cause an out-of-bounds memory
> write where the offset and the value are controlled by guest.
>
> This will corrupt QEMU memory, in most situations this causes the vm to
> crash.
>
> Fix by checking the offset against the array size.
>
> Reported-by: =E6=9D=8E=E5=BC=BA <liqiang6-s@360.cn>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index 486591b..7977878 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -375,6 +375,9 @@ static void uart_write(void *opaque, hwaddr offset,
>
>      DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value=
);
>      offset >>=3D 2;
> +    if (offset >=3D CADENCE_UART_R_MAX) {
> +        return;
> +    }
>      switch (offset) {
>      case R_IER: /* ier (wts imr) */
>          s->r[R_IMR] |=3D value;

thanks
-- PMM