linux-kernel - Re: [PATCH] powerpc/pseries: Fix of_node_put() underflow during pseries remove

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Jul 2017 10:53:23 -0700
From:   Tyrel Datwyler <turtle.in.the.kernel@...il.com>
To:     Michael Ellerman <mpe@...erman.id.au>,
        Laurent Vivier <lvivier@...hat.com>,
        linux-kernel@...r.kernel.org
Cc:     David Gibson <david@...son.dropbear.id.au>,
        linuxppc-dev@...ts.ozlabs.org,
        Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>,
        Thomas Huth <thuth@...hat.com>
Subject: Re: [PATCH] powerpc/pseries: Fix of_node_put() underflow during
 pseries remove

On 07/24/2017 09:47 PM, Michael Ellerman wrote:
> Tyrel Datwyler <turtle.in.the.kernel@...il.com> writes:
> 
>> On 07/24/2017 03:42 AM, Michael Ellerman wrote:
>>> Laurent Vivier <lvivier@...hat.com> writes:
>>>
>>>> As for commit 68baf692c435 ("powerpc/pseries: Fix of_node_put()
>>>> underflow during DLPAR remove"), the call to of_node_put()
>>>> must be removed from pSeries_reconfig_remove_node().
>>>>
>>>> dlpar_detach_node() and pSeries_reconfig_remove_node() call
>>>> of_detach_node(), and thus the node should not be released
>>>> in this case too.
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@...hat.com>
>>>> ---
>>>>  arch/powerpc/platforms/pseries/reconfig.c | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>
>>> Thanks. I'll spare you the swearing about why we have the same bug in
>>> two places.
>>
>> That's probably my bad. I must have failed to test with older powerpc-util tooling where
>> drmgr uses the /proc/ofdt interface for device tree modification.
> 
> OK. Really we should have automated tests of the various cases, I've
> just never had time to write any.

Agreed, some better CI is warranted.

> 
> Mainly the thing that bugs me is that we still have the two separate
> paths. Or if we must maintain both they could at least share more code,
> the two functions do basically the same thing AFAICS.

Yeah, I think that is where I dropped the ball. I wrongly assumed by not looking close
enough that code was shared in those two paths. Definitely some code de-duplication work
that can be done.

-Tyrel

> 
> cheers
>