diff mbox series

[v2,1/3] s390/vfio-ap: unpin pages on gisc registration failure

Message ID 20231018133829.147226-2-akrowiak@linux.ibm.com
State New
Headers
Series a couple of corrections to the IRQ enablement function |

Commit Message

Anthony Krowiak Oct. 18, 2023, 1:38 p.m. UTC 
  From: Anthony Krowiak <akrowiak@linux.ibm.com>

In the vfio_ap_irq_enable function, after the page containing the
notification indicator byte (NIB) is pinned, the function attempts
to register the guest ISC. If registration fails, the function sets the
status response code and returns without unpinning the page containing
the NIB. In order to avoid a memory leak, the NIB should be unpinned before
returning from the vfio_ap_irq_enable function.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")
Cc: <stable@vger.kernel.org>
---
 drivers/s390/crypto/vfio_ap_ops.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Christian Borntraeger Oct. 26, 2023, 12:18 p.m. UTC | #1 
Am 18.10.23 um 15:38 schrieb Tony Krowiak:
> From: Anthony Krowiak <akrowiak@linux.ibm.com>
> 
> In the vfio_ap_irq_enable function, after the page containing the
> notification indicator byte (NIB) is pinned, the function attempts
> to register the guest ISC. If registration fails, the function sets the
> status response code and returns without unpinning the page containing
> the NIB. In order to avoid a memory leak, the NIB should be unpinned before
> returning from the vfio_ap_irq_enable function.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Where is Janoschs signed off coming from here?

> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/s390/crypto/vfio_ap_ops.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 4db538a55192..9cb28978c186 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -457,6 +457,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
>   		VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
>   				 __func__, nisc, isc, q->apqn);
>   
> +		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>   		status.response_code = AP_RESPONSE_INVALID_GISA;
>   		return status;
>   	}
Anthony Krowiak Oct. 26, 2023, 1:16 p.m. UTC | #2 
On 10/26/23 08:18, Christian Borntraeger wrote:
> 
> 
> Am 18.10.23 um 15:38 schrieb Tony Krowiak:
>> From: Anthony Krowiak <akrowiak@linux.ibm.com>
>>
>> In the vfio_ap_irq_enable function, after the page containing the
>> notification indicator byte (NIB) is pinned, the function attempts
>> to register the guest ISC. If registration fails, the function sets the
>> status response code and returns without unpinning the page containing
>> the NIB. In order to avoid a memory leak, the NIB should be unpinned 
>> before
>> returning from the vfio_ap_irq_enable function.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> 
> Where is Janoschs signed off coming from here?

Janosch found this and composed the patch originally. I just tweaked the 
description and posted it.

> 
>> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
>> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the 
>> vfio_ap_irq_enable function")
>> Cc: <stable@vger.kernel.org>
>> ---
>>   drivers/s390/crypto/vfio_ap_ops.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 4db538a55192..9cb28978c186 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -457,6 +457,7 @@ static struct ap_queue_status 
>> vfio_ap_irq_enable(struct vfio_ap_queue *q,
>>           VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, 
>> isc=%d, apqn=%#04x\n",
>>                    __func__, nisc, isc, q->apqn);
>> +        vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>>           status.response_code = AP_RESPONSE_INVALID_GISA;
>>           return status;
>>       }
Christian Borntraeger Oct. 26, 2023, 1:25 p.m. UTC | #3 
Am 26.10.23 um 15:16 schrieb Tony Krowiak:
> 
> 
> On 10/26/23 08:18, Christian Borntraeger wrote:
>>
>>
>> Am 18.10.23 um 15:38 schrieb Tony Krowiak:
>>> From: Anthony Krowiak <akrowiak@linux.ibm.com>
>>>
>>> In the vfio_ap_irq_enable function, after the page containing the
>>> notification indicator byte (NIB) is pinned, the function attempts
>>> to register the guest ISC. If registration fails, the function sets the
>>> status response code and returns without unpinning the page containing
>>> the NIB. In order to avoid a memory leak, the NIB should be unpinned before
>>> returning from the vfio_ap_irq_enable function.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>
>> Where is Janoschs signed off coming from here?
> 
> Janosch found this and composed the patch originally. I just tweaked the description and posted it.

So we should add

Co-developed-by: Janosch Frank <frankja@linux.ibm.com>

in front of Janoschs signoff.

> 
>>
>>> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>>> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>   drivers/s390/crypto/vfio_ap_ops.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>>> index 4db538a55192..9cb28978c186 100644
>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>>> @@ -457,6 +457,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
>>>           VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
>>>                    __func__, nisc, isc, q->apqn);
>>> +        vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>>>           status.response_code = AP_RESPONSE_INVALID_GISA;
>>>           return status;
>>>       }
Anthony Krowiak Oct. 26, 2023, 2:20 p.m. UTC | #4 
On 10/26/23 09:25, Christian Borntraeger wrote:
> 
> 
> Am 26.10.23 um 15:16 schrieb Tony Krowiak:
>>
>>
>> On 10/26/23 08:18, Christian Borntraeger wrote:
>>>
>>>
>>> Am 18.10.23 um 15:38 schrieb Tony Krowiak:
>>>> From: Anthony Krowiak <akrowiak@linux.ibm.com>
>>>>
>>>> In the vfio_ap_irq_enable function, after the page containing the
>>>> notification indicator byte (NIB) is pinned, the function attempts
>>>> to register the guest ISC. If registration fails, the function sets the
>>>> status response code and returns without unpinning the page containing
>>>> the NIB. In order to avoid a memory leak, the NIB should be unpinned 
>>>> before
>>>> returning from the vfio_ap_irq_enable function.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>
>>> Where is Janoschs signed off coming from here?
>>
>> Janosch found this and composed the patch originally. I just tweaked 
>> the description and posted it.
> 
> So we should add
> 
> Co-developed-by: Janosch Frank <frankja@linux.ibm.com>
> 
> in front of Janoschs signoff.

Will do.

> 
>>
>>>
>>>> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>>>> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>>> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the 
>>>> vfio_ap_irq_enable function")
>>>> Cc: <stable@vger.kernel.org>
>>>> ---
>>>>   drivers/s390/crypto/vfio_ap_ops.c | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
>>>> b/drivers/s390/crypto/vfio_ap_ops.c
>>>> index 4db538a55192..9cb28978c186 100644
>>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>>>> @@ -457,6 +457,7 @@ static struct ap_queue_status 
>>>> vfio_ap_irq_enable(struct vfio_ap_queue *q,
>>>>           VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, 
>>>> isc=%d, apqn=%#04x\n",
>>>>                    __func__, nisc, isc, q->apqn);
>>>> +        vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>>>>           status.response_code = AP_RESPONSE_INVALID_GISA;
>>>>           return status;
>>>>       }
diff mbox series

Patch

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 4db538a55192..9cb28978c186 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -457,6 +457,7 @@  static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
 		VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
 				 __func__, nisc, isc, q->apqn);
 
+		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
 		status.response_code = AP_RESPONSE_INVALID_GISA;
 		return status;
 	}