linux-kernel - Re: [PATCH 1/3] driver core: class: Fix wild pointer dereference in API class_dev_iter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7ecd5828-5e7a-4882-87ad-9c5fbf274524@icloud.com>
Date: Wed, 13 Nov 2024 20:39:09 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, linux-kernel@...r.kernel.org,
 Zijun Hu <quic_zijuhu@...cinc.com>, stable@...r.kernel.org
Subject: Re: [PATCH 1/3] driver core: class: Fix wild pointer dereference in
 API class_dev_iter_next()

On 2024/11/12 22:57, Greg Kroah-Hartman wrote:
> 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@...cinc.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@...r.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.
> 

Hi Greg.

This fix is very worthy and necessary since it fixes the APIs self issue
and the issue is irrelevant with how various API users (in-tree or
out-of-tree) use it as below inference shown:

API class_dev_iter_init() has checks for class_to_subsys(class) error
-> the error may happen
-> once the error happens
-> wild pointers dereference must happen within API class_dev_iter_next()
-> how terrible for such issue, and the error handling also have no
prompt messages.

API users must not like silent wild pointer dereference once the error
happen.

> thanks,
> 
> greg k-h