Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: sidgup=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org CFD49C433CA
Subject: Re: PROBLEM: Firmware loader fallback mechanism no longer works with
 sendfile
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     mcgrof@kernel.org, rafael@kernel.org, viro@zeniv.linux.org.uk,
        linux-fsdevel@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "psodagud@codeaurora.org" <psodagud@codeaurora.org>
References: <7e6f44b1-a0d2-d1d1-9c11-dcea163f8f03@codeaurora.org>
 <X/QJCgoLPhfECEmP@kroah.com>
 <180bdfaf-8c84-6946-b46f-3729d4eb17cc@codeaurora.org>
 <X/WSA7nmsUSrpsfr@kroah.com>
From:   Siddharth Gupta <sidgup@codeaurora.org>
Message-ID: <62583aaa-d557-8c9a-5959-52c9efad1fe3@codeaurora.org>
Date:   Thu, 7 Jan 2021 14:03:47 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: <X/WSA7nmsUSrpsfr@kroah.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


On 1/6/2021 2:33 AM, Greg KH wrote:
> On Tue, Jan 05, 2021 at 05:00:58PM -0800, Siddharth Gupta wrote:
>> On 1/4/2021 10:36 PM, Greg KH wrote:
>>> On Mon, Jan 04, 2021 at 02:43:45PM -0800, Siddharth Gupta wrote:
>>>> Hi all,
>>>>
>>>> With the introduction of the filesystem change "fs: don't allow splice
>>>> read/write without explicit ops"[1] the fallback mechanism of the firmware
>>>> loader[2] no longer works when using sendfile[3] from the userspace.
>>> What userspace program are you using to load firmware?
>> The userspace program is in the android userspace which listens to a uevent
>> from the firmware loader and then loads the firmware using sendfile[1].
>>>    Are you not using the in-kernel firmware loader for some reason?
>> We have certain non-standard firmware paths that should not be added to the
>> linux kernel, and the firmware_class.path only supports a single path.
> That option is just for a single override, which should be all that you
> need if the other paths that are built into the kernel do not work.
> Surely one of the 5 different paths here are acceptable?
Unfortunately they are not, and we understand that such changes 
shouldn't make it to upstream hence it was not a part of the request. If 
the firmware loader fallback mechanism was being deprecated then we 
would have to look into our options. As of now the series of changes 
breaking the sysfs bin attributes is the only bug that affects us.
>
> If not, how many more do you need?
We need 2 paths.
>
> And last I looked, Android wants you to use the built-in kernel firmware
> loader, and NOT an external firmware binary anymore.  So this shouldn't
> be an issue for your newer systems anyway :)
In our discussion with the Android team that is not the case currently. 
In the future yes, but not now :)

Regardless this bug is in the kernel and not Android. If the firmware 
loader fallback mechanism is used on the current kernel we would see the 
issue with sendfile in the userspace whether Android is running or not.
>
>>>> Since the binary attributes don't support splice_{read,write} functions the
>>>> calls to splice_{read,write} used the default kernel_{read,write} functions.
>>>> With the above change this results in an -EINVAL return from
>>>> do_splice_from[4].
>>>>
>>>> This essentially means that sendfile will not work for any binary attribute
>>>> in the sysfs.
>>> Have you tried fixing this with a patch much like what we did for the
>>> proc files that needed this?  If not, can you?
>> I am not aware of this fix, could you provide me a link for reference? I
>> will try it out.
> Look at the series of commits starting at fe33850ff798 ("proc: wire up
> generic_file_splice_read for iter ops") for how this was fixed in procfs
> as an example of what also needs to be done for binary sysfs files.
I tried to follow these fixes, but I am unfamiliar with fs code. I don't 
see the generic_file_splice_write function anymore on newer kernels, 
also AFAICT kernfs_ops does not define {read,write}_iter operations. If 
the solution is simple and someone could provide the patches I would be 
happy to test them out. If not, some more information about how to 
proceed would be nice.

Thanks,
Sid
>
> thanks,
>
> greg k-h