linux-kernel - Re: PROBLEM: Firmware loader fallback mechanism no longer works with sendfile

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:   Tue, 12 Jan 2021 10:31:26 -0800
From:   Siddharth Gupta <sidgup@...eaurora.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     mcgrof@...nel.org, rafael@...nel.org, viro@...iv.linux.org.uk,
        linux-fsdevel@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "psodagud@...eaurora.org" <psodagud@...eaurora.org>
Subject: Re: PROBLEM: Firmware loader fallback mechanism no longer works with
 sendfile


On 1/8/2021 6:44 AM, Greg KH wrote:
> On Thu, Jan 07, 2021 at 02:03:47PM -0800, Siddharth Gupta wrote:
>> On 1/6/2021 2:33 AM, Greg KH wrote:
>>>>>> 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.
> Can you try this tiny patch out below?
Sorry for the delay, I tried out the patch, but I am still seeing the 
error. Please take a look at these logs with
android running in the userspace[1]:

[   62.295056][  T249] remoteproc remoteproc1: powering up 
xxxxxxxx.remoteproc-cdsp
[   62.304138][  T249] remoteproc remoteproc1: Direct firmware load for 
cdsp.mdt failed with error -2
[   62.312976][  T249] remoteproc remoteproc1: Falling back to sysfs 
fallback for: cdsp.mdt
[   62.469748][  T394] ueventd: firmware: loading 'cdsp.mdt' for 
'/devices/platform/soc/xxxxxxxx.remoteproc-cdsp/remoteproc/remoteproc1/cdsp.mdt'
[   62.498700][  T394] ueventd: firmware: sendfile failed { 
'/sys/devices/platform/soc/xxxxxxxx.remoteproc-cdsp/remoteproc/remoteproc1/cdsp.mdt', 
'cdsp.mdt' }: Invalid argument

Thanks,
Sid

[1]: 
https://android.googlesource.com/platform/system/core/+/refs/heads/master/init/firmware_handler.cpp#57
>
> thanks,
>
> greg k-h
>
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index f277d023ebcd..113bc816d430 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -968,6 +968,8 @@ const struct file_operations kernfs_file_fops = {
>   	.release	= kernfs_fop_release,
>   	.poll		= kernfs_fop_poll,
>   	.fsync		= noop_fsync,
> +	.splice_read	= generic_file_splice_read,
> +	.splice_write	= iter_file_splice_write,
>   };
>   
>   /**