KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Tony Krowiak <akrowiak@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, freude@linux.ibm.com,
	borntraeger@de.ibm.com, frankja@linux.ibm.com, david@redhat.com,
	mjrosato@linux.ibm.com, schwidefsky@de.ibm.com,
	heiko.carstens@de.ibm.com, pmorel@linux.ibm.com,
	pasic@linux.ibm.com, alex.williamson@redhat.com,
	kwankhede@nvidia.com
Subject: Re: [PATCH v4 3/7] s390: zcrypt: driver callback to indicate resource in use
Date: Wed, 19 Jun 2019 09:04:18 -0400
Message-ID: <2366c6b6-fd9e-0c32-0e9d-018cd601a0ad@linux.ibm.com> (raw)
In-Reply-To: <20190618182558.7d7e025a.cohuck@redhat.com>

On 6/18/19 12:25 PM, Cornelia Huck wrote:
> On Thu, 13 Jun 2019 15:39:36 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
>> Introduces a new driver callback to prevent a root user from unbinding
>> an AP queue from its device driver if the queue is in use. This prevents
>> a root user from inadvertently taking a queue away from a guest and
>> giving it to the host, or vice versa. The callback will be invoked
>> whenever a change to the AP bus's apmask or aqmask sysfs interfaces may
>> result in one or more AP queues being removed from its driver. If the
>> callback responds in the affirmative for any driver queried, the change
>> to the apmask or aqmask will be rejected with a device in use error.
>>
>> For this patch, only non-default drivers will be queried. Currently,
>> there is only one non-default driver, the vfio_ap device driver. The
>> vfio_ap device driver manages AP queues passed through to one or more
>> guests and we don't want to unexpectedly take AP resources away from
>> guests which are most likely independently administered.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   drivers/s390/crypto/ap_bus.c | 138 +++++++++++++++++++++++++++++++++++++++++--
>>   drivers/s390/crypto/ap_bus.h |   3 +
>>   2 files changed, 135 insertions(+), 6 deletions(-)
> 
> Hm... I recall objecting to this patch before, fearing that it makes it
> possible for a bad actor to hog resources that can't be removed by
> root, even forcefully. (I have not had time to look at the intervening
> versions, so I might be missing something.)
> 
> Is there a way for root to forcefully override this?

You recall correctly; however, after many internal crypto team
discussions, it was decided that this feature was important
and should be kept.

Allow me to first address your fear that a bad actor can hog
resources that can't be removed by root. With this enhancement,
there is nothing preventing a root user from taking resources
from a matrix mdev, it simply forces him/her to follow the
proper procedure. The resources to be removed must first be
unassigned from the matrix mdev to which they are assigned.
The AP bus's /sys/bus/ap/apmask and /sys/bus/ap/aqmask
sysfs attributes can then be edited to transfer ownership
of the resources to zcrypt.

The rationale for keeping this feature is:

* It is a bad idea to steal an adapter in use from a guest. In the worst
   case, the guest could end up without access to any crypto adapters
   without knowing why. This could lead to performance issues on guests
   that rely heavily on crypto such as guests used for blockchain
   transactions.

* There are plenty of examples in linux of the kernel preventing a root
   user from performing a task. For example, a module can't be removed
   if references are still held for it. Another example would be trying
   to bind a CEX4 adapter to a device driver not registered for CEX4;
   this action will also be rejected.

* The semantics are much cleaner and the logic is far less complicated.

* It forces the use of the proper procedure to change ownership of AP
   queues.


> 


  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13 19:39 [PATCH v4 0/7] s390: vfio-ap: dynamic configuration support Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 1/7] s390: vfio-ap: Refactor vfio_ap driver probe and remove callbacks Tony Krowiak
2019-06-17  8:27   ` Harald Freudenberger
2019-06-17 14:24     ` Tony Krowiak
2019-06-18 16:14   ` Cornelia Huck
2019-06-19 12:31     ` Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 2/7] s390: vfio-ap: wait for queue empty on queue reset Tony Krowiak
2019-06-17  8:47   ` Harald Freudenberger
2019-06-17 14:29     ` Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 3/7] s390: zcrypt: driver callback to indicate resource in use Tony Krowiak
2019-06-17  9:28   ` Harald Freudenberger
2019-06-17 14:37     ` Tony Krowiak
2019-06-18 16:25   ` Cornelia Huck
2019-06-19 13:04     ` Tony Krowiak [this message]
2019-06-26 21:13       ` Tony Krowiak
2019-06-27  7:25         ` Cornelia Huck
2019-06-27 12:59           ` Tony Krowiak
2019-07-01 19:26       ` Cornelia Huck
2019-07-08 14:27         ` Tony Krowiak
2019-07-09 10:49           ` Cornelia Huck
2019-07-09 21:11             ` Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 4/7] s390: vfio-ap: implement in-use callback for vfio_ap driver Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 5/7] s390: vfio-ap: allow assignment of unavailable AP resources to mdev device Tony Krowiak
2019-06-17 10:05   ` Harald Freudenberger
2019-06-17 15:07     ` Tony Krowiak
2019-06-18  6:49       ` Harald Freudenberger
2019-06-19 13:39         ` Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 6/7] s390: vfio-ap: allow hot plug/unplug of AP resources using " Tony Krowiak
2019-06-13 19:39 ` [PATCH v4 7/7] s390: vfio-ap: update documentation Tony Krowiak
2019-06-17 11:42   ` Harald Freudenberger
2019-06-17 15:21     ` Tony Krowiak
2019-07-09 15:30 ` [PATCH v4 0/7] s390: vfio-ap: dynamic configuration support Halil Pasic

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2366c6b6-fd9e-0c32-0e9d-018cd601a0ad@linux.ibm.com \
    --to=akrowiak@linux.ibm.com \
    --cc=alex.williamson@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=freude@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    --cc=pmorel@linux.ibm.com \
    --cc=schwidefsky@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox