From: Dan Carpenter <error27@gmail.com>
To: hanzhijian <hanzhijian1991@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Viresh Kumar <vireshk@kernel.org>,
Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: greybus: bootrom: fix potential NULL dereference
Date: Fri, 21 Aug 2026 14:53:13 +0300 [thread overview]
Message-ID: <aog8KROHA18VIZtm@stanley.mountain> (raw)
In-Reply-To: <20260821113540.1989561-1-hanzhijian1991@gmail.com>
You're using the word "potential" but the commit message correctly
explains why a NULL dereference is impossible. Don't say potentially
for things which are impossible.
On Fri, Aug 21, 2026 at 07:35:40PM +0800, hanzhijian wrote:
> In gb_bootrom_get_firmware(), the queue_work label dereferences fw->size
> on a path where fw may have been set to NULL via the "if (!fw) goto
> unlock" path. This is currently masked at runtime by the !ret
> short-circuit (ret is non-zero on every path where fw can be NULL), but
> it relies on an implicit invariant that is fragile and hard to follow.
A lot of people would argue that the original code is easy to follow. In
your code, to see what is passed on error you have to scroll all the way
to the top of the function to see the "next_request =
NEXT_REQ_GET_FIRMWARE;" assignment. In the existing code, it's clear,
this is what we pass on error, this is what we pass on success.
It's not really fragile either. If we screwed up and forgot to set the
error code or something then Smatch would warn about that.
drivers/staging/greybus/bootrom.c:300 gb_bootrom_get_firmware() error: we previously assumed 'fw' could be null (see line 266)
Or on the earlier paths, we would get an uninitialized variable
warning.
regards,
dan carpenter
next prev parent reply other threads:[~2026-08-21 11:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-21 11:35 hanzhijian
2026-08-21 11:53 ` Dan Carpenter [this message]
2026-08-21 12:07 ` hanzhijian
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=aog8KROHA18VIZtm@stanley.mountain \
--to=error27@gmail.com \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=hanzhijian1991@gmail.com \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=vireshk@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®