From: Shuah Khan <skhan@linuxfoundation.org>
To: Anirudh Rayabharam <mail@anirudhrb.com>,
mcgrof@kernel.org, gregkh@linuxfoundation.org, rafael@kernel.org
Cc: linux-kernel-mentees@lists.linuxfoundation.org,
linux-kernel@vger.kernel.org,
syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Subject: Re: [PATCH v8 2/2] firmware_loader: fix use-after-free in firmware_fallback_sysfs
Date: Wed, 28 Jul 2021 10:37:38 -0600 [thread overview]
Message-ID: <a3c433b2-3bda-67cc-1ec3-220e4530a12a@linuxfoundation.org> (raw)
In-Reply-To: <20210728085107.4141-3-mail@anirudhrb.com>
On 7/28/21 2:51 AM, Anirudh Rayabharam wrote:
> This use-after-free happens when a fw_priv object has been freed but
> hasn't been removed from the pending list (pending_fw_head). The next
> time fw_load_sysfs_fallback tries to insert into the list, it ends up
> accessing the pending_list member of the previously freed fw_priv.
>
> The root cause here is that all code paths that abort the fw load
> don't delete it from the pending list. For example:
>
> _request_firmware()
> -> fw_abort_batch_reqs()
> -> fw_state_aborted()
>
> To fix this, delete the fw_priv from the list in __fw_set_state() if
> the new state is DONE or ABORTED. This way, all aborts will remove
> the fw_priv from the list. Accordingly, remove calls to list_del_init
> that were being made before calling fw_state_(aborted|done).
>
> Also, in fw_load_sysfs_fallback, don't add the fw_priv to the pending
> list if it is already aborted. Instead, just jump out and return early.
>
> Fixes: bcfbd3523f3c ("firmware: fix a double abort case with fw_load_sysfs_fallback")
> Reported-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
> Tested-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
> Acked-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
> ---
> drivers/base/firmware_loader/fallback.c | 12 ++++++++----
> drivers/base/firmware_loader/firmware.h | 10 +++++++++-
> drivers/base/firmware_loader/main.c | 2 ++
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
Thank you. Looks good.
Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
thanks,
-- Shuah
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
WARNING: multiple messages have this Message-ID (diff)
From: Shuah Khan <skhan@linuxfoundation.org>
To: Anirudh Rayabharam <mail@anirudhrb.com>,
mcgrof@kernel.org, gregkh@linuxfoundation.org, rafael@kernel.org
Cc: linux-kernel@vger.kernel.org,
linux-kernel-mentees@lists.linuxfoundation.org,
syzbot+de271708674e2093097b@syzkaller.appspotmail.com,
Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v8 2/2] firmware_loader: fix use-after-free in firmware_fallback_sysfs
Date: Wed, 28 Jul 2021 10:37:38 -0600 [thread overview]
Message-ID: <a3c433b2-3bda-67cc-1ec3-220e4530a12a@linuxfoundation.org> (raw)
In-Reply-To: <20210728085107.4141-3-mail@anirudhrb.com>
On 7/28/21 2:51 AM, Anirudh Rayabharam wrote:
> This use-after-free happens when a fw_priv object has been freed but
> hasn't been removed from the pending list (pending_fw_head). The next
> time fw_load_sysfs_fallback tries to insert into the list, it ends up
> accessing the pending_list member of the previously freed fw_priv.
>
> The root cause here is that all code paths that abort the fw load
> don't delete it from the pending list. For example:
>
> _request_firmware()
> -> fw_abort_batch_reqs()
> -> fw_state_aborted()
>
> To fix this, delete the fw_priv from the list in __fw_set_state() if
> the new state is DONE or ABORTED. This way, all aborts will remove
> the fw_priv from the list. Accordingly, remove calls to list_del_init
> that were being made before calling fw_state_(aborted|done).
>
> Also, in fw_load_sysfs_fallback, don't add the fw_priv to the pending
> list if it is already aborted. Instead, just jump out and return early.
>
> Fixes: bcfbd3523f3c ("firmware: fix a double abort case with fw_load_sysfs_fallback")
> Reported-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
> Tested-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
> Acked-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
> ---
> drivers/base/firmware_loader/fallback.c | 12 ++++++++----
> drivers/base/firmware_loader/firmware.h | 10 +++++++++-
> drivers/base/firmware_loader/main.c | 2 ++
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
Thank you. Looks good.
Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
thanks,
-- Shuah
next prev parent reply other threads:[~2021-07-28 16:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-28 8:51 [PATCH v8 0/2] firmware_loader: fix uaf in firmware_fallback_sysfs Anirudh Rayabharam
2021-07-28 8:51 ` Anirudh Rayabharam
2021-07-28 8:51 ` [PATCH v8 1/2] firmware_loader: use -ETIMEDOUT instead of -EAGAIN in fw_load_sysfs_fallback Anirudh Rayabharam
2021-07-28 8:51 ` Anirudh Rayabharam
2021-07-28 8:51 ` [PATCH v8 2/2] firmware_loader: fix use-after-free in firmware_fallback_sysfs Anirudh Rayabharam
2021-07-28 8:51 ` Anirudh Rayabharam
2021-07-28 16:37 ` Shuah Khan [this message]
2021-07-28 16:37 ` Shuah Khan
2021-07-28 20:37 ` [PATCH v8 0/2] firmware_loader: fix uaf " Luis Chamberlain
2021-07-28 20:37 ` Luis Chamberlain
2021-07-29 16:52 ` Anirudh Rayabharam
2021-07-29 16:52 ` 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=a3c433b2-3bda-67cc-1ec3-220e4530a12a@linuxfoundation.org \
--to=skhan@linuxfoundation.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mail@anirudhrb.com \
--cc=mcgrof@kernel.org \
--cc=rafael@kernel.org \
--cc=syzbot+de271708674e2093097b@syzkaller.appspotmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.