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

From: Macpaul Lin <macpaul.lin@mediatek.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Sasha Levin <sashal@kernel.org>, Shen Jing <jingx.shen@intel.com>,
	CC Hwang <cc.hwang@mediatek.com>,
	Mediatek WSD Upstream <wsd_upstream@mediatek.com>,
	Jerry Zhang <zhangjerry@google.com>,
	andreyknvl@google.com, linux-usb@vger.kernel.org,
	Loda Chou <loda.chou@mediatek.com>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	Miles Chen <miles.chen@mediatek.com>,
	linux-mediatek@lists.infradead.org,
	Peter Chen <peter.chen@nxp.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Vincent Pelletier <plr.vincent@gmail.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	John Stultz <john.stultz@linaro.org>,
	eugenis@google.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode
Date: Sun, 1 Mar 2020 11:20:43 +0800	[thread overview]
Message-ID: <1583032843.12083.24.camel@mtkswgap22> (raw)
In-Reply-To: <20200228164848.GH4019108@arrakis.emea.arm.com>

On Fri, 2020-02-28 at 16:48 +0000, Catalin Marinas wrote:
> On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin wrote:
> > 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;
> 
> I had forgotten that we discussed a similar case already a few months
> ago (thanks to Evgenii for pointing out). Do you have this commit
> applied to your tree: df325e05a682 ("arm64: Validate tagged addresses in
> access_ok() called from kernel threads")?
> 

Yes! We have that patch. I've also got Google's reply about referencing
this patch in android kernel tree.
https://android-review.googlesource.com/c/kernel/common/+/1186615

However, during my debugging process, I've dumped specific length (e.g.,
24 bytes for the first request) AIO request buffer address both in adbd
and in __range_ok(). Then I've found __range_ok() still always return
false on address begin with "0x3c". Since untagged_addr() already called
in __range_ok(), to set "TIF_TAGGED_ADDR" with adbd's user space buffer
should be the possible solution. Hence I've send the v3 patch.

Anyway, I've found that to disable TAGGED address in adbd is possible by
this way and will report to Google and see how they think.

diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp
index 9e02e89ab..b2f6f8e3f 100644
--- a/adb/daemon/main.cpp
+++ b/adb/daemon/main.cpp
@@ -317,6 +317,8 @@ int main(int argc, char** argv) {
     mallopt(M_DECAY_TIME, 1);
 #endif

+    prctl(PR_SET_TAGGED_ADDR_CTRL, ~PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
+
     while (true) {
         static struct option opts[] = {
                 {"root_seclabel", required_argument, nullptr, 's'},

Many thanks!
Macpaul Lin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
