linux-kernel - Re: [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]

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

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