linux-kernel - Re: [PATCH] platform: release resource itself instead of resource tree

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]

Date:   Fri, 26 Jul 2019 16:01:18 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Chang, Junxiao" <junxiao.chang@...el.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "Li, Lili" <lili.li@...el.com>
Subject: Re: [PATCH] platform: release resource itself instead of resource
 tree

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Fri, Jul 26, 2019 at 10:24:22AM +0000, Chang, Junxiao wrote:
> Hi Greg,
> 
> Thank you for looking at it.
> 
> Below example is simplified description. Our detail problem is:
> 1. ACPI driver registers a MEM resource during bootup;
> 2. Our PUNIT(Intel CPU power management module) platform device reads ACPI driver's resource, and registers same MEM resource;
> 3. According to current resource management logic, if two resources are same, later registered resource will be parent. That is, PUNIT platform device's resource will be ACPI driver resource's parent.
> 4. PUNIT kernel module is removed, its resource will be removed. If we use original API "release_resource", ACPI driver's resource will be released as well because it is PUNIT device's child;

Why did you remove this module?  That never happens unless you do it "by
hand", and as root.  Just don't do this if it causes problems in your
system, right?

Anyway, if you create a child reference you should always clean up all
of your children before removing yourself from memory.  So try fixing
that up first.

> 5. Next time PUNIT platform device kernel module is inserted, it might read wrong ACPI MEM resource because it has been released in step 4.
> 
> How should we handle this case? :) 

Don't unload kernel modules unless you know what you are doing :)

Seriously, you are creating a dependancy here that you are not
expressing in your module reference count, and you are not properly
cleaning up after yourself when removing your devices.  Just delete your
child devices when unloading yourself and you should be fine, right?

> We should not register same MEM resource in step 2? Or, make change in
> resource management logic, if two resources are same, later registered
> resource should be child instead of parent?

I don't know how your resource management logic works, try working with
the developers who wrote that.  But as you describe it, it is not
correct at least when you try to clean things up.

thanks,

greg k-h