From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751824AbdKTQ1Z (ORCPT <rfc822;w@1wt.eu>);
        Mon, 20 Nov 2017 11:27:25 -0500
Received: from userp1040.oracle.com ([156.151.31.81]:35610 "EHLO
        userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751523AbdKTQ1Y (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 20 Nov 2017 11:27:24 -0500
Date: Mon, 20 Nov 2017 11:26:52 -0500
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Eric Yang <yu.yang_3@nxp.com>, iommu@lists.linux-foundation.org
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrey Ryabinin <aryabinin@virtuozzo.com>,
        David Miller <davem@davemloft.net>, Ingo Molnar <mingo@kernel.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Al Viro <viro@zeniv.linux.org.uk>, Kees Cook <keescook@chromium.org>,
        Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: No check of the size passed to unmap_single in  swiotlb
Message-ID: <20171120162652.GR24312@char.us.oracle.com>
References: <VI1PR04MB30715EA7212DC661481041438C220@VI1PR04MB3071.eurprd04.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <VI1PR04MB30715EA7212DC661481041438C220@VI1PR04MB3071.eurprd04.prod.outlook.com>
User-Agent: Mutt/1.8.3 (2017-05-23)
X-Source-IP: userv0022.oracle.com [156.151.31.74]
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Nov 20, 2017 at 08:17:14AM +0000, Eric Yang wrote:
> Hi all,

Hi!
> 
> During debug a  device only support 32bits DMA(Qualcomm Atheros AP) in our LS1043A 64bits ARM  SOC, we found that the invoke of dma_unmap_single --> swiotlb_tbl_unmap_single  will unmap the passed "size" anyway even when the "size" is incorrect.
> 
> If the size is larger than it should, the extra entries in io_tlb_orig_addr array will be refilled by INVALID_PHYS_ADDR, and it will cause the bounce buffer copy not happen when the one who really used the mis-freed entries doing  DMA data transfers, and this will cause further unknow behaviors.
> 
> Here we just fix it temporarily by adding a judge of the "size" in the swiotlb_tbl_unmap_single, if it is larger than it deserves, just unmap the right size only. Like the code:

Did the DMA debug API (CONFIG_DMA_API_DEBUG) help in figuring this issue as well?

> 
> [yangyu@titan dash-lts]$ git diff ./lib/swiotlb.c
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index ad1d2962d129..58c97ede9d78 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -591,7 +591,10 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
>                  */
>                 for (i = index + nslots - 1; i >= index; i--) {
>                         io_tlb_list[i] = ++count;
> -                       io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
> +                      if(io_tlb_orig_addr[i]  != orig_addr)
> +                          printk("======size wrong, ally down ally down!===\n");
> +                      else
> +                         io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
>                 }
>                 /*
>                  * Step 2: merge the returned slots with the preceding slots,
> 
> Although pass a right size of DMA buffer is the responsibility of  the drivers, but Is it useful to add some size check code to prevent  real damage happen? 
> 
> Regards,
> Eric