linux-kernel - No check of the size passed to unmap

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <VI1PR04MB30715EA7212DC661481041438C220@VI1PR04MB3071.eurprd04.prod.outlook.com>
Date:   Mon, 20 Nov 2017 08:17:14 +0000
From:   Eric Yang <yu.yang_3@....com>
To:     Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        "David Miller" <davem@...emloft.net>,
        Ingo Molnar <mingo@...nel.org>,
        "Geert Uytterhoeven" <geert+renesas@...der.be>,
        Al Viro <viro@...iv.linux.org.uk>,
        Kees Cook <keescook@...omium.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: No check of the size passed to unmap_single in  swiotlb

Hi all,

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:

[yangyu@...an 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