linux-kernel - Re: [PATCH v2 7/8] s390: vfio-ap: handle bind and unbind of AP queue device

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: <20190423155458.65966ebf.pasic@linux.ibm.com>
Date:   Tue, 23 Apr 2019 15:54:58 +0200
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     Tony Krowiak <akrowiak@...ux.ibm.com>
Cc:     linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, freude@...ux.ibm.com, borntraeger@...ibm.com,
        cohuck@...hat.com, frankja@...ux.ibm.com, david@...hat.com,
        schwidefsky@...ibm.com, heiko.carstens@...ibm.com,
        pmorel@...ux.ibm.com, alex.williamson@...hat.com,
        kwankhede@...dia.com
Subject: Re: [PATCH v2 7/8] s390: vfio-ap: handle bind and unbind of AP
 queue device

On Sat, 20 Apr 2019 17:49:39 -0400
Tony Krowiak <akrowiak@...ux.ibm.com> wrote:

> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
> +
> +	/*
> +	 * If the queue is assigned to the mdev device and the mdev device
> +	 * is in use by a guest
> +	 */
> +	if (matrix_mdev && matrix_mdev->kvm) {
> +		/* Plug the adapter into the guest */
> +		set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
> +
> +		/* Make sure the queue is also plugged in to the guest */
> +		if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
> +			set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
> +
> +		vfio_ap_mdev_update_crycb(matrix_mdev);

With this you effectively grant access to all the assigned domains on
the AP identified by the apid, not only to the domain identified by
apqi! But some of these queues may still not be bound to the vfio_ap
driver.

IMHO you should only set the apid-th bit in apm if all queues (apid, q)
such that q-th bit is set in aqm are bound to the vfio_ap driver.

BTW a 'shadow' (or effective) apm would perfectly suffice. I don't think
you fiddle with shadow_crycb->a[qd]m, and if you do, I don't think that's
a good idea.

Regards,
Halil

> +	}
> +}