From: Dan Carpenter <dan.carpenter@oracle.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Russ Weight <russell.h.weight@intel.com>
Cc: Tianfei zhang <tianfei.zhang@intel.com>,
Luis Chamberlain <mcgrof@kernel.org>,
Lee Jones <lee.jones@linaro.org>,
Shawn Guo <shawn.guo@linaro.org>,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [PATCH v3] test_firmware: fix end of loop test in upload_read_show()
Date: Fri, 6 May 2022 09:55:15 +0300 [thread overview]
Message-ID: <YnTGU3UJOIA09I7e@kili> (raw)
If a list_for_each_entry() loop exits without hitting a break statement
then the iterator points to invalid memory. So in this code the
"tst->name" dereference is an out bounds read. It's an offset from the
&test_upload_list pointer and it will likely work fine most of the time
but it's not correct.
One alternative is to fix this this by changing the test to:
if (list_entry_is_head(tst, &test_upload_list, node)) {
But the simpler, trendy new way is just create a new variable and test
for NULL.
Fixes: a31ad463b72d ("test_firmware: Add test support for firmware upload")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Russ Weight <russell.h.weight@intel.com>
---
v3: Add Russ's Reviewed-by tag and this --- revision text
v2: Change the commit message
lib/test_firmware.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 76115c1a2629..c82b65947ce6 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -1392,7 +1392,8 @@ static ssize_t upload_read_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- struct test_firmware_upload *tst;
+ struct test_firmware_upload *tst = NULL;
+ struct test_firmware_upload *tst_iter;
int ret = -EINVAL;
if (!test_fw_config->upload_name) {
@@ -1401,11 +1402,13 @@ static ssize_t upload_read_show(struct device *dev,
}
mutex_lock(&test_fw_mutex);
- list_for_each_entry(tst, &test_upload_list, node)
- if (tst->name == test_fw_config->upload_name)
+ list_for_each_entry(tst_iter, &test_upload_list, node)
+ if (tst_iter->name == test_fw_config->upload_name) {
+ tst = tst_iter;
break;
+ }
- if (tst->name != test_fw_config->upload_name) {
+ if (!tst) {
pr_err("Firmware name not found: %s\n",
test_fw_config->upload_name);
goto out;
--
2.35.1
next reply other threads:[~2022-05-06 6:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-06 6:55 Dan Carpenter [this message]
2022-05-09 22:37 ` [PATCH v3] test_firmware: fix end of loop test in upload_read_show() Luis Chamberlain
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=YnTGU3UJOIA09I7e@kili \
--to=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=russell.h.weight@intel.com \
--cc=shawn.guo@linaro.org \
--cc=tianfei.zhang@intel.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.