linux-kernel - Re: [REGRESSION] usb: gadget: f_fs: Allow scatter-gather buffers

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: <1672b93a-dfe6-acb2-715e-c4a13af54413@collabora.com>
Date:   Tue, 21 May 2019 12:04:49 +0200
From:   Andrzej Pietrasiewicz <andrzej.p@...labora.com>
To:     "Yang, Fei" <fei.yang@...el.com>,
        John Stultz <john.stultz@...aro.org>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Chen Yu <chenyu56@...wei.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Linux USB List <linux-usb@...r.kernel.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        "kernel@...labora.com" <kernel@...labora.com>
Subject: Re: [REGRESSION] usb: gadget: f_fs: Allow scatter-gather buffers

Hi,

W dniu 20.05.2019 o 23:52, Yang, Fei pisze:
>>>>> One question that comes to my mind is this: Does the USB

<snip>

> 
> One of the problems appears to be that req->num_mapped_sgs was left uninitialized. I made the following change and got a lot more requests completed.
> However this change is not sufficient to solve the adb issue, the usb requests would eventually get stuck without getting a matching ffs_epfile_async_io_complete.
> 
> @@ -1067,6 +1067,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
>                          req->buf = NULL;
>                          req->sg = io_data->sgt.sgl;
>                          req->num_sgs = io_data->sgt.nents;
> +                       req->num_mapped_sgs = req->num_sgs;
>                  } else {
>                          req->buf = data;
>                  }
> @@ -1110,6 +1111,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
>                          req->buf = NULL;
>                          req->sg = io_data->sgt.sgl;
>                          req->num_sgs = io_data->sgt.nents;
> +                       req->num_mapped_sgs = req->num_sgs;
>                  } else {
>                          req->buf = data;
>                  }
> 

Isn't num_mapped_sgs meant to be set by drivers/usb/gadget/udc/core.c?

And the comment in include/linux/usb/gadget.h says "internal".

One thing that becomes evident now is that adb uses async io.
It seems that interaction of async io and s-g mode should be further
investigated.

Andrzej