From mboxrd@z Thu Jan  1 00:00:00 1970
From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Subject: Re: [PATCH 4/8] nvdimm acpi: implement Read FIT function
Date: Fri, 15 Jul 2016 15:43:25 +0800
Message-ID: <5788941D.1030509@linux.intel.com>
References: <1468244718-3731-1-git-send-email-guangrong.xiao@linux.intel.com>
 <1468244718-3731-5-git-send-email-guangrong.xiao@linux.intel.com>
 <20160714121750.GL15476@stefanha-x1.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: pbonzini@redhat.com, imammedo@redhat.com, gleb@kernel.org,
	mtosatti@redhat.com, mst@redhat.com, rth@twiddle.net,
	ehabkost@redhat.com, dan.j.williams@intel.com, kvm@vger.kernel.org,
	qemu-devel@nongnu.org
To: Stefan Hajnoczi <stefanha@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mga03.intel.com ([134.134.136.65]:59342 "EHLO mga03.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752560AbcGOHr3 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 15 Jul 2016 03:47:29 -0400
In-Reply-To: <20160714121750.GL15476@stefanha-x1.localdomain>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 07/14/2016 08:17 PM, Stefan Hajnoczi wrote:

>> +/* Read FIT data, defined in docs/specs/acpi_nvdimm.txt. */
>> +static void nvdimm_dsm_func_read_fit(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
>> +{
>> +    NvdimmFuncReadFITIn *read_fit;
>> +    NvdimmFuncReadFITOut *read_fit_out;
>> +    GSList *device_list = nvdimm_get_plugged_device_list();
>> +    GArray *fit = nvdimm_build_device_structure(device_list);
>> +    uint32_t read_len = 0, func_ret_status;
>> +    int left, size;
>> +
>> +    read_fit = (NvdimmFuncReadFITIn *)in->arg3;
>> +    le32_to_cpus(&read_fit->offset);
>> +
>> +    nvdimm_debug("Read FIT: offset %#x FIT size %#x.\n", read_fit->offset,
>> +                 fit->len);
>> +
>> +    left = fit->len - read_fit->offset;
>> +    if (left < 0) {
>
> Signed integer overflow leads to memory disclosure in memcpy() below.
> The problem occurs when (guint)fit->len - (uint32_t)read_fit->offset >
> INT_MAX.
>
> Please perform the check like this:
>
>    if (fit->offset >= fit->len) {
>

Ah, yes, you are right, thank you for pointing it out. Will fix it.