From: Luis Chamberlain <email@example.com> To: Shuah Khan <firstname.lastname@example.org> Cc: Anirudh Rayabharam <email@example.com>, Hillf Danton <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Subject: Re: [PATCH] firmware_loader: Fix use-after-free Read in firmware_loading_store Date: Thu, 15 Jul 2021 16:21:05 -0700 [thread overview] Message-ID: <20210715232105.am4wsxfclj2ufjdw@garbanzo> (raw) In-Reply-To: <email@example.com> On Thu, Jul 15, 2021 at 04:46:24PM -0600, Shuah Khan wrote: > On 7/15/21 4:28 PM, Luis Chamberlain wrote: > > On Fri, Jul 09, 2021 at 10:38:12AM -0600, Shuah Khan wrote: > > > However I am seeing the following over and over again in the > > > log - hence I think it is safer to check the aborted status > > > in __fw_load_abort(). > > > > > > ? __list_del_entry_valid+0xe0/0xf0 > > > [ 348.604808][T12994] __list_del_entry_valid+0xe0/0xf0 > > > [ 348.610020][T12994] firmware_loading_store+0x141/0x650 > > > [ 348.615761][T12994] ? firmware_data_write+0x4e0/0x4e0 > > > [ 348.621064][T12994] ? sysfs_file_ops+0x1c0/0x1c0 > > > [ 348.625921][T12994] dev_attr_store+0x50/0x80 > > > > > > Also the fallback logic takes actions based on errors as in > > > fw_load_sysfs_fallback() that returns -EAGAIN which would > > > trigger request_firmware() again. > > > > > > Based on all of this I think this fix is needed, if only I can > > > test for sure. > > > > Shuah, curious if you had read this patch from Anirudh Rayabharam > > and my response to that v4 patch iteration? > > > > https://firstname.lastname@example.org > > > > Yes. I realized I am trying to fix the same problem we have been > discussing. :) Sorry for the noise. No worries, and thanks again for you help! > Ignore my patch. I will follow the thread. OK ! I think all we need is just Anirudh to split his patch to remove the -EAGAIN return value in a separate patch as a first step, documenting in the commmit log that: The only motivation on her part with using -EAGAIN on commit 0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val for fw load abort") was to distinguish the error from -ENOMEM, and so there is no real reason in keeping it. Keeping -ETIMEDOUT is much telling of what the reason for a failure is, so just use that. Then his second patch would be simplified without the -EAGAIN condition. All I asked was to confirm that the -ETIMEDOUT was indeed propagated. Anirudh, sorry for the trouble, but can I ask you for a v5 with two patches as described above? Luis
next prev parent reply other threads:[~2021-07-15 23:21 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-08 3:13 Shuah Khan [not found] ` <email@example.com> 2021-07-09 16:15 ` Shuah Khan 2021-07-09 16:38 ` Shuah Khan 2021-07-15 22:28 ` Luis Chamberlain 2021-07-15 22:46 ` Shuah Khan 2021-07-15 23:21 ` Luis Chamberlain [this message] 2021-07-18 9:01 ` Anirudh Rayabharam
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210715232105.am4wsxfclj2ufjdw@garbanzo \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] firmware_loader: Fix use-after-free Read in firmware_loading_store' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).