linux-kernel - [PATCH v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode

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]

Message-ID: <1582718512-28923-1-git-send-email-macpaul.lin@mediatek.com>
Date:   Wed, 26 Feb 2020 20:01:52 +0800
From:   Macpaul Lin <macpaul.lin@...iatek.com>
To:     Matthias Brugger <matthias.bgg@...il.com>,
        Shen Jing <jingx.shen@...el.com>,
        Sasha Levin <sashal@...nel.org>,
        John Stultz <john.stultz@...aro.org>,
        Andrzej Pietrasiewicz <andrzej.p@...labora.com>,
        Vincent Pelletier <plr.vincent@...il.com>,
        Jerry Zhang <zhangjerry@...gle.com>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>,
        Mediatek WSD Upstream <wsd_upstream@...iatek.com>,
        CC Hwang <cc.hwang@...iatek.com>,
        Loda Chou <loda.chou@...iatek.com>,
        Al Viro <viro@...iv.linux.org.uk>, <stable@...r.kernel.org>,
        <andreyknvl@...gle.com>
CC:     Macpaul Lin <macpaul.lin@...iatek.com>,
        Peter Chen <peter.chen@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Miles Chen <miles.chen@...iatek.com>
Subject: [PATCH v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode

This issue was found when adbd trying to open functionfs with AIO mode.
Usually, we need to set "setprop sys.usb.ffs.aio_compat 0" to enable
adbd with AIO mode on Android.

When adbd is opening functionfs, it will try to read 24 bytes at the
first read I/O control. If this reading has been failed, adbd will
try to send FUNCTIONFS_CLEAR_HALT to functionfs. When adbd is in AIO
mode, functionfs will be acted with asyncronized I/O path. After the
successful read transfer has been completed by gadget hardware, the
following series of functions will be called.
  ffs_epfile_async_io_complete() -> ffs_user_copy_worker() ->
    copy_to_iter() -> _copy_to_iter() -> copyout() ->
    iterate_and_advance() -> iterate_iovec()

Adding debug trace to these functions, it has been found that in
copyout(), access_ok() will check if the user space address is valid
to write. However if CONFIG_ARM64_TAGGED_ADDR_ABI is enabled, adbd
always passes user space address start with "0x3C" to gadget's AIO
blocks. This tagged address will cause access_ok() check always fail.
Which causes later calculation in iterate_iovec() turn zero.
Copyout() won't copy data to user space since the length to be copied
"v.iov_len" will be zero. Finally leads ffs_copy_to_iter() always return
-EFAULT, causes adbd cannot open functionfs and send
FUNCTIONFS_CLEAR_HALT.

Signed-off-by: Macpaul Lin <macpaul.lin@...iatek.com>
Cc: Peter Chen <peter.chen@....com>
Cc: Catalin Marinas <catalin.marinas@....com>
Cc: Miles Chen <miles.chen@...iatek.com>
---
Changes for v4:
  - Abandon solution v3 by adding "TIF_TAGGED_ADDR" flag to gadget driver.
    According to Catalin's suggestion, change the solution by untagging 
    user space address passed by AIO in gadget driver.

Changes for v3:
  - Fix misspelling in commit message.
    Thanks for Peter's review.

Changes for v2:
  - Fix build error for 32-bit load. An #if defined(CONFIG_ARM64) still need
    for avoiding undeclared defines.

 drivers/usb/gadget/function/f_fs.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index ce1d023..192935f 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req)
 
 static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter)
 {
-	ssize_t ret = copy_to_iter(data, data_len, iter);
+	ssize_t ret;
+
+#if defined(CONFIG_ARM64)
+	/*
+	 * Replace tagged address passed by user space application before
+	 * copying.
+	 */
+	if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) &&
+		(iter->type == ITER_IOVEC)) {
+		*(unsigned long *)&iter->iov->iov_base =
+			(unsigned long)untagged_addr(iter->iov->iov_base);
+	}
+#endif
+	ret = copy_to_iter(data, data_len, iter);
 	if (likely(ret == data_len))
 		return ret;
 
-- 
1.7.9.5