Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] powerpc/pseries: Fix dn reference error in
 dlpar_cpu_remove_by_index
To:     Tyrel Datwyler <tyreld@linux.vnet.ibm.com>,
        linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Cc:     Rob Herring <robh@kernel.org>,
        Thomas Falcon <tlfalcon@linux.vnet.ibm.com>,
        Nicholas Piggin <npiggin@gmail.com>,
        Paul Mackerras <paulus@samba.org>,
        Nathan Lynch <nathanl@linux.vnet.ibm.com>,
        "Juliet M. Kim" <minkim@linux.vnet.ibm.com>
References: <cc20c5cc-cf8e-d125-cac9-5d1b938a1d86@linux.vnet.ibm.com>
 <a7986146-f09d-6d40-63f9-8e7307fc2609@linux.vnet.ibm.com>
From:   Michael Bringmann <mwb@linux.vnet.ibm.com>
Openpgp: preference=signencrypt
Autocrypt: addr=mwb@linux.vnet.ibm.com; prefer-encrypt=mutual; keydata=
 mQENBFcY7GcBCADzw3en+yzo9ASFGCfldVkIg95SAMPK0myXp2XJYET3zT45uBsX/uj9/2nA
 lBmXXeOSXnPfJ9V3vtiwcfATnWIsVt3tL6n1kqikzH9nXNxZT7MU/7gqzWZngMAWh/GJ9qyg
 DTOZdjsvdUNUWxtiLvBo7y+reA4HjlQhwhYxxvCpXBeRoF0qDWfQ8DkneemqINzDZPwSQ7zY
 t4F5iyN1I9GC5RNK8Y6jiKmm6bDkrrbtXPOtzXKs0J0FqWEIab/u3BDrRP3STDVPdXqViHua
 AjEzthQbGZm0VCxI4a7XjMi99g614/qDcXZCs00GLZ/VYIE8hB9C5Q+l66S60PLjRrxnABEB
 AAG0LU1pY2hhZWwgVy4gQnJpbmdtYW5uIDxtd2JAbGludXgudm5ldC5pYm0uY29tPokBOAQT
 AQIAIgUCVxjsZwIbAwYLCQgHAwIGFQgCCQoLBBYCAwECHgECF4AACgkQSEdag3dpuTI0NAf8
 CKYTDKQLgOSjVrU2L5rM4lXaJRmQV6oidD3vIhKSnWRvPq9C29ifRG6ri20prTHAlc0vycgm
 41HHg0y2vsGgNXGTWC2ObemoZBI7mySXe/7Tq5mD/semGzOp0YWZ7teqrkiSR8Bw0p+LdE7K
 QmT7tpjjvuhrtQ3RRojUYcuy1nWUsc4D+2cxsnZslsx84FUKxPbLagDgZmgBhUw/sUi40s6S
 AkdViVCVS0WANddLIpG0cfdsV0kCae/XdjK3mRK6drFKv1z+QFjvOhc8QIkkxFD0da9w3tJj
 oqnqHFV5gLcHO6/wizPx/NV90y6RngeBORkQiRFWxTXS4Oj9GVI/UrkBDQRXGOxnAQgAmJ5Y
 ikTWrMWPfiveUacETyEhWVl7u8UhZcx3yy2te8O0ay7t9fYcZgIEfQPPVVus89acIXlG3wYL
 DDPvb21OprLxi+ZJ2a0S5we+LcSWN1jByxJlbWBq+/LcMtGAOhNLpysY1gD0Y4UW/eKS+TFZ
 562qKC3k1dBvnV9JXCgeS1taYFxRdVAn+2DwK3nuyG/DDq/XgJ5BtmyC3MMx8CiW3POj+O+l
 6SedIeAfZlZ7/xhijx82g93h07VavUQRwMZgZFsqmuxBxVGiav2HB+dNvs3PFB087Pvc9OHe
 qhajPWOP/gNLMmvBvknn1NToM9a8/E8rzcIZXoYs4RggRRYh6wARAQABiQEfBBgBAgAJBQJX
 GOxnAhsMAAoJEEhHWoN3abky+RUH/jE08/r5QzaNKYeVhu0uVbgXu5fsxqr2cAxhf+KuwT3T
 efhEP2alarxzUZdEh4MsG6c+X2NYLbD3cryiXxVx/7kSAJEFQJfA5P06g8NLR25Qpq9BLsN7
 ++dxQ+CLKzSEb1X24hYAJZpOhS8ev3ii+M/XIo+olDBKuTaTgB6elrg3CaxUsVgLBJ+jbRkW
 yQe2S5f/Ja1ThDpSSLLWLiLK/z7+gaqwhnwjQ8Z8Y9D2itJQcj4itHilwImsqwLG7SxzC0NX
 IQ5KaAFYdRcOgwR8VhhkOIVd70ObSZU+E4pTET1WDz4o65xZ89yfose1No0+r5ht/xWOOrh8
 53/hcWvxHVs=
Organization: IBM Linux Technology Center
Date:   Wed, 20 Feb 2019 14:18:56 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <a7986146-f09d-6d40-63f9-8e7307fc2609@linux.vnet.ibm.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Message-Id: <8bc3e665-828f-8247-0778-56d7ee045e53@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2/19/19 2:03 PM, Tyrel Datwyler wrote:
> On 02/19/2019 07:46 AM, Michael Bringmann wrote:
>> powerpc/pseries: Fix dn reference error in dlpar_cpu_remove_by_index()
>>
>> A reference to the device node of the CPU to be removed is released
>> upon successful removal of the associated CPU device.  If the call
>> to remove the CPU device fails, dlpar_cpu_remove_by_index() still
>> frees the reference and this leads to miscomparisons and/or
>> addressing errors later on.
>>
>> This problem may be observed when trying to DLPAR 'hot-remove' a CPU
>> from a system that has only a single CPU.  The operation will fail
>> because there is no other CPU to which the kernel operations may be
>> migrated, but the refcount will still be decremented.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>>
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> index 97feb6e..9537bb9 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -635,7 +635,8 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>>  	}
>>
>>  	rc = dlpar_cpu_remove(dn, drc_index);
>> -	of_node_put(dn);
>> +	if (!rc)
>> +		of_node_put(dn);
>>  	return rc;
>>  }
>>
> 
> NACK!
> 
> The logic here is wrong. Here is the full function.
> 
> static int dlpar_cpu_remove_by_index(u32 drc_index)
> {
>         struct device_node *dn;
>         int rc;
> 
>         dn = cpu_drc_index_to_dn(drc_index);
>         if (!dn) {
>                 pr_warn("Cannot find CPU (drc index %x) to remove\n",
>                         drc_index);
>                 return -ENODEV;
>         }
> 
>         rc = dlpar_cpu_remove(dn, drc_index);
>         of_node_put(dn);
>         return rc;
> }
> 
> The call to cpu_drc_index_to_dn() returns a device_node with the reference count
> incremented. So, regardless of the success or failure of the call to
> dlpar_cpu_remove() you need to release that reference.
> 
> If there is a reference counting issue it is somewhere else.

Okay.  Withdrawn while we look some more.

> -Tyrel

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com