From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Zijun Hu <zijun_hu@icloud.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
linux-kernel@vger.kernel.org, Zijun Hu <quic_zijuhu@quicinc.com>,
stable@vger.kernel.org
Subject: Re: [PATCH 1/3] driver core: class: Fix wild pointer dereference in API class_dev_iter_next()
Date: Tue, 12 Nov 2024 15:57:31 +0100 [thread overview]
Message-ID: <2024111230-erratic-clay-7565@gregkh> (raw)
In-Reply-To: <2952f37a-7a11-42d9-9b90-4856ed200610@icloud.com>
On Tue, Nov 12, 2024 at 10:46:27PM +0800, Zijun Hu wrote:
> On 2024/11/12 19:43, Greg Kroah-Hartman wrote:
> > On Tue, Nov 05, 2024 at 08:20:22AM +0800, Zijun Hu wrote:
> >> From: Zijun Hu <quic_zijuhu@quicinc.com>
> >>
> >> class_dev_iter_init(struct class_dev_iter *iter, struct class *class, ...)
> >> has return type void, but it does not initialize its output parameter @iter
> >> when suffers class_to_subsys(@class) error, so caller can not detect the
> >> error and call API class_dev_iter_next(@iter) which will dereference wild
> >> pointers of @iter's members as shown by below typical usage:
> >>
> >> // @iter's members are wild pointers
> >> struct class_dev_iter iter;
> >>
> >> // No change in @iter when the error happens.
> >> class_dev_iter_init(&iter, ...);
> >>
> >> // dereference these wild member pointers here.
> >> while (dev = class_dev_iter_next(&iter)) { ... }.
> >>
> >> Actually, all callers of the API have such usage pattern in kernel tree.
> >> Fix by memset() @iter in API *_init() and error checking @iter in *_next().
> >>
> >> Fixes: 7b884b7f24b4 ("driver core: class.c: convert to only use class_to_subsys")
> >> Cc: stable@vger.kernel.org
> >
> > There is no in-kernel broken users of this from what I can tell, right?
> > Otherwise things would have blown up by now, so why is this needed in
> > stable kernels?
> >
>
> For all callers of the API in current kernel tree, the class should have
> been registered successfully when the API is invoking.
Great, so the existing code is just fine :)
> so, could you remove both Fix and stable tag directly?
Nope, sorry. Asking a maintainer that gets hundreds of patches to
hand-edit them does not scale.
But really, as all in-kernel users are just fine, why add additional
code if it's not needed? THat's just going to increase our maintance
burden for the next 40+ years for no good reason.
thanks,
greg k-h
next prev parent reply other threads:[~2024-11-12 14:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-05 0:20 [PATCH 0/3] driver core: class: Fix bug and code improvements for class APIs Zijun Hu
2024-11-05 0:20 ` [PATCH 1/3] driver core: class: Fix wild pointer dereference in API class_dev_iter_next() Zijun Hu
2024-11-12 11:43 ` Greg Kroah-Hartman
2024-11-12 14:46 ` Zijun Hu
2024-11-12 14:57 ` Greg Kroah-Hartman [this message]
2024-11-12 15:05 ` Zijun Hu
2024-11-13 12:39 ` Zijun Hu
2024-11-05 0:20 ` [PATCH 2/3] driver core: class: Correct WARN() message in APIs class_(for_each|find)_device() Zijun Hu
2024-11-05 0:20 ` [PATCH 3/3] driver core: class: Delete a redundant check " Zijun Hu
2024-11-12 11:45 ` Greg Kroah-Hartman
2024-11-12 14:51 ` Zijun Hu
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=2024111230-erratic-clay-7565@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_zijuhu@quicinc.com \
--cc=rafael@kernel.org \
--cc=stable@vger.kernel.org \
--cc=zijun_hu@icloud.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox