From: Macpaul Lin <macpaul.lin@mediatek.com>
To: Matthias Brugger <matthias.bgg@gmail.com>,
Shen Jing <jingx.shen@intel.com>, Sasha Levin <sashal@kernel.org>,
John Stultz <john.stultz@linaro.org>,
Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
Vincent Pelletier <plr.vincent@gmail.com>,
Jerry Zhang <zhangjerry@google.com>, <linux-usb@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>,
Mediatek WSD Upstream <wsd_upstream@mediatek.com>,
CC Hwang <cc.hwang@mediatek.com>,
Loda Chou <loda.chou@mediatek.com>,
Al Viro <viro@zeniv.linux.org.uk>, <stable@vger.kernel.org>,
<andreyknvl@google.com>
Cc: Peter Chen <peter.chen@nxp.com>,
Miles Chen <miles.chen@mediatek.com>,
Macpaul Lin <macpaul.lin@mediatek.com>,
Catalin Marinas <catalin.marinas@arm.com>
Subject: [PATCH v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode
Date: Wed, 26 Feb 2020 20:01:52 +0800 [thread overview]
Message-ID: <1582718512-28923-1-git-send-email-macpaul.lin@mediatek.com> (raw)
In-Reply-To: <1582627315-21123-1-git-send-email-macpaul.lin@mediatek.com>
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@mediatek.com>
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Miles Chen <miles.chen@mediatek.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
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Macpaul Lin <macpaul.lin@mediatek.com>
To: Matthias Brugger <matthias.bgg@gmail.com>,
Shen Jing <jingx.shen@intel.com>, Sasha Levin <sashal@kernel.org>,
John Stultz <john.stultz@linaro.org>,
Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
Vincent Pelletier <plr.vincent@gmail.com>,
Jerry Zhang <zhangjerry@google.com>, <linux-usb@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>,
Mediatek WSD Upstream <wsd_upstream@mediatek.com>,
CC Hwang <cc.hwang@mediatek.com>,
Loda Chou <loda.chou@mediatek.com>,
Al Viro <viro@zeniv.linux.org.uk>, <stable@vger.kernel.org>,
<andreyknvl@google.com>
Cc: Macpaul Lin <macpaul.lin@mediatek.com>,
Peter Chen <peter.chen@nxp.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Miles Chen <miles.chen@mediatek.com>
Subject: [PATCH v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode
Date: Wed, 26 Feb 2020 20:01:52 +0800 [thread overview]
Message-ID: <1582718512-28923-1-git-send-email-macpaul.lin@mediatek.com> (raw)
In-Reply-To: <1582627315-21123-1-git-send-email-macpaul.lin@mediatek.com>
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@mediatek.com>
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Miles Chen <miles.chen@mediatek.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
WARNING: multiple messages have this Message-ID (diff)
From: Macpaul Lin <macpaul.lin@mediatek.com>
To: Matthias Brugger <matthias.bgg@gmail.com>,
Shen Jing <jingx.shen@intel.com>, Sasha Levin <sashal@kernel.org>,
John Stultz <john.stultz@linaro.org>,
Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
Vincent Pelletier <plr.vincent@gmail.com>,
Jerry Zhang <zhangjerry@google.com>, <linux-usb@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>,
Mediatek WSD Upstream <wsd_upstream@mediatek.com>,
CC Hwang <cc.hwang@mediatek.com>,
Loda Chou <loda.chou@mediatek.com>,
Al Viro <viro@zeniv.linux.org.uk>, <stable@vger.kernel.org>,
<andreyknvl@google.com>
Cc: Peter Chen <peter.chen@nxp.com>,
Miles Chen <miles.chen@mediatek.com>,
Macpaul Lin <macpaul.lin@mediatek.com>,
Catalin Marinas <catalin.marinas@arm.com>
Subject: [PATCH v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode
Date: Wed, 26 Feb 2020 20:01:52 +0800 [thread overview]
Message-ID: <1582718512-28923-1-git-send-email-macpaul.lin@mediatek.com> (raw)
In-Reply-To: <1582627315-21123-1-git-send-email-macpaul.lin@mediatek.com>
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@mediatek.com>
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Miles Chen <miles.chen@mediatek.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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-02-26 12:02 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-23 15:49 [PATCH v2] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode Macpaul Lin
2020-02-23 15:49 ` Macpaul Lin
2020-02-23 15:49 ` Macpaul Lin
2020-02-25 8:12 ` Peter Chen
2020-02-25 8:12 ` Peter Chen
2020-02-25 8:12 ` Peter Chen
2020-02-25 10:41 ` [PATCH v3] " Macpaul Lin
2020-02-25 10:41 ` Macpaul Lin
2020-02-25 10:41 ` Macpaul Lin
2020-02-25 11:07 ` Miles Chen
2020-02-25 11:07 ` Miles Chen
2020-02-25 11:07 ` Miles Chen
2020-02-25 11:52 ` Catalin Marinas
2020-02-25 11:52 ` Catalin Marinas
2020-02-25 11:52 ` Catalin Marinas
2020-02-26 12:01 ` Macpaul Lin [this message]
2020-02-26 12:01 ` [PATCH v4] " Macpaul Lin
2020-02-26 12:01 ` Macpaul Lin
2020-02-27 9:55 ` Catalin Marinas
2020-02-27 9:55 ` Catalin Marinas
2020-02-27 9:55 ` Catalin Marinas
2020-02-27 10:28 ` Macpaul Lin
2020-02-27 10:28 ` Macpaul Lin
2020-02-27 10:28 ` Macpaul Lin
2020-02-28 16:48 ` Catalin Marinas
2020-02-28 16:48 ` Catalin Marinas
2020-02-28 16:48 ` Catalin Marinas
2020-03-01 3:20 ` Macpaul Lin
2020-03-01 3:20 ` Macpaul Lin
2020-03-01 3:20 ` Macpaul Lin
2020-03-02 16:19 ` Catalin Marinas
2020-03-02 16:19 ` Catalin Marinas
2020-03-02 16:19 ` Catalin Marinas
[not found] ` <CAFKCwrj-0aQN_cUxf8=h7AbfS_rLEwxqePZN2kGHZxgWi2=ncw@mail.gmail.com>
2020-03-04 2:42 ` Macpaul Lin
2020-03-04 2:42 ` Macpaul Lin
2020-03-04 2:42 ` Macpaul Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1582718512-28923-1-git-send-email-macpaul.lin@mediatek.com \
--to=macpaul.lin@mediatek.com \
--cc=andreyknvl@google.com \
--cc=andrzej.p@collabora.com \
--cc=catalin.marinas@arm.com \
--cc=cc.hwang@mediatek.com \
--cc=jingx.shen@intel.com \
--cc=john.stultz@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-usb@vger.kernel.org \
--cc=loda.chou@mediatek.com \
--cc=matthias.bgg@gmail.com \
--cc=miles.chen@mediatek.com \
--cc=peter.chen@nxp.com \
--cc=plr.vincent@gmail.com \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=wsd_upstream@mediatek.com \
--cc=zhangjerry@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.