linux-kernel - Re: [PATCH] s390: vfio-ap: disable IRQ in remove callback results in kernel OOPS

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]

Message-ID: <20191029231917.0e6a62b9.cohuck@redhat.com>
Date:   Tue, 29 Oct 2019 23:19:17 +0100
From:   Cornelia Huck <cohuck@...hat.com>
To:     Tony Krowiak <akrowiak@...ux.ibm.com>
Cc:     linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, heiko.carstens@...ibm.com, gor@...ux.ibm.com,
        borntraeger@...ibm.com, freude@...ux.ibm.com,
        mjrosato@...ux.ibm.com, pmorel@...ux.ibm.com, pasic@...ux.ibm.com,
        jjherne@...ux.ibm.com
Subject: Re: [PATCH] s390: vfio-ap: disable IRQ in remove callback results
 in kernel OOPS

On Tue, 29 Oct 2019 18:09:06 -0400
Tony Krowiak <akrowiak@...ux.ibm.com> wrote:

> From: aekrowia <akrowiak@...ux.ibm.com>

Some accident seems to have happened to your git config.

> 
> When an AP adapter card is configured off via the SE or the SCLP
> Deconfigure Adjunct Processor command and the AP bus subsequently detects
> that the adapter card is no longer in the AP configuration, the card
> device representing the adapter card as well as each of its associated
> AP queue devices will be removed by the AP bus. If one or more of the
> affected queue devices is bound to the VFIO AP device driver, its remove
> callback will be invoked for each queue to be removed. The remove callback
> resets the queue and disables IRQ processing. If interrupt processing was
> never enabled for the queue, disabling IRQ processing will fail resulting
> in a kernel OOPS.
> 
> This patch verifies IRQ processing is enabled before attempting to disable
> interrupts for the queue.
> 
> Signed-off-by: Tony Krowiak <akrowiak@...ux.ibm.com>
> Signed-off-by: aekrowia <akrowiak@...ux.ibm.com>
> ---
>  drivers/s390/crypto/vfio_ap_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
> index be2520cc010b..42d8308fd3a1 100644
> --- a/drivers/s390/crypto/vfio_ap_drv.c
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -79,7 +79,8 @@ static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
>  	apid = AP_QID_CARD(q->apqn);
>  	apqi = AP_QID_QUEUE(q->apqn);
>  	vfio_ap_mdev_reset_queue(apid, apqi, 1);
> -	vfio_ap_irq_disable(q);
> +	if (q->saved_isc != VFIO_AP_ISC_INVALID)
> +		vfio_ap_irq_disable(q);

Hm... would it make sense to move that check into vfio_ap_irq_disable()
instead? Or are we sure that in all other cases the irq processing had
been enabled before?

Also, if that oops is reasonably easy to trigger, it would probably
make sense to cc:stable. (Or is this a new problem?)

>  	kfree(q);
>  	mutex_unlock(&matrix_dev->lock);
>  }