Re: [PATCH v7 1/6] s390/vfio-ap: Fix leaks of pinned NIB and registered GISC

[Christian Borntraeger <borntraeger@linux.ibm.com>]

Am 22.09.26 um 16:29 schrieb Anthony Krowiak:
> 
> 
> On 9/4/26 6:51 PM, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Critical] Eagerly freeing AQIC resources when ap_zapq() or ap_tapq() returns an unrecognized response allows wild DMA writes to freed host memory.
>> -- 
>>
>> commit 6c73d9ea4d311d5d68e8f5501374842cccbe2c90
>> Author: Anthony Krowiak <akrowiak@linux.ibm.com>
>>
>> s390/vfio-ap: Fix leaks of pinned NIB and registered GISC
>>
>> Several code paths in the vfio_ap driver failed to free the AQIC
>> resources — the pinned guest NIB page and the registered guest ISC
>> used to enable interrupts for a queue — when a queue became unavailable
>> or when unexpected response codes were returned. This could cause memory
>> exhaustion and depletion of KVM interrupt subclass registrations over time
>> with repeated dynamic AP reconfiguration.
>>
>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>>> index 940c0ff668bee..9d4f5b9e3301f 100644
>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> [ ... ]
>>> @@ -1959,8 +2087,16 @@ static void apq_reset_check(struct work_struct *reset_work)
>>>           elapsed += AP_RESET_INTERVAL;
>>>           status = ap_tapq(q->apqn, NULL);
>>>           ret = apq_status_check(q->apqn, &status);
>>> -        if (ret == -EIO)
>>> +        if (ret == -EIO) {
>>> +            /*
>>> +             * TAPQ returned an invalid response code. This
>>> +             * indicates a hardware or firmware bug; the queue
>>> +             * cannot generate AP interrupts or DMA-write to the
>>> +             * NIB, so free the AQIC resources rather than leak them.
>>> +             */
>>> +            vfio_ap_free_aqic_resources(q);
>>>               return;
>>> +        }
>> [Severity: Critical]
>> If apq_status_check() returns -EIO due to an unrecognized TAPQ response,
>> is it safe to eagerly free the AQIC resources?
>>
>> If the hardware is in an unknown state (due to a bug or unrecognized code),
>> could it still be actively DMA-writing to the pinned Notification Indicator
>> Byte (NIB) page?
>>
>> If vfio_ap_free_aqic_resources() unpins the page and returns it to the
>> system allocator, could a subsequent wild DMA write from the malfunctioning
>> AP queue corrupt memory belonging to a new owner, potentially causing a host
>> panic?
> 
> Since we don't know the state of the queue, the AQIC resources should be
> intentionally leaked here lest the NIB page be re-allocated to another
> process potentially resulting in a wild DMA-write.

Yes, we should not free the ressources and rather leak them to avoid reuse.