linux-kernel - Re: [PATCH 0/3] cxl/mbox: support background operation abort requests

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] [day] [month] [year] [list]

Message-ID: <750047084f2e456bb639fabfa7189b20@micron.com>
Date: Thu, 24 Oct 2024 06:30:12 +0000
From: Ravis OpenSrc <Ravis.OpenSrc@...ron.com>
To: Davidlohr Bueso <dave@...olabs.net>
CC: "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"dave.jiang@...el.com" <dave.jiang@...el.com>, "jonathan.cameron@...wei.com"
	<jonathan.cameron@...wei.com>, "alison.schofield@...el.com"
	<alison.schofield@...el.com>, "vishal.l.verma@...el.com"
	<vishal.l.verma@...el.com>, "ira.weiny@...el.com" <ira.weiny@...el.com>,
	"fan.ni@...sung.com" <fan.ni@...sung.com>, "a.manzanares@...sung.com"
	<a.manzanares@...sung.com>, Srinivasulu Opensrc
	<sthanneeru.opensrc@...ron.com>, Eishan Mirakhur <emirakhur@...ron.com>,
	"Ajay Joshi" <ajayjoshi@...ron.com>, Srinivasulu Thanneeru
	<sthanneeru@...ron.com>, "linux-cxl@...r.kernel.org"
	<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] cxl/mbox: support background operation abort requests

>On Wed, 23 Oct 2024, Davidlohr Bueso wrote:
>>The one major functionality in our original proposal apart from implementing abort is
>>
>>Allowing background commands to be invoked from user space through the primary mailbox
>>by ensuring only those background commands are enabled which also support request abort operation
>>by checking their individual CEL details.
>
>Is vendor-specific logs not something that can be done OoB?
>
>If we are strictly talking about CEL details, yes this series could use that, and was
>planning on adding it for an eventual v2 -- ie: why send the abort cmd at all if we
>know the current one doesn't support it. But that is minutiae, for kernel bg commands.
>
>But yeah, for your needs, the enabled cmds would need that filter.

Ok. we can merge changes related to CEL checking and filtering of enabled commands.

>
>Then with that, would adding something like the below address your needs and below
>questions? Basically, if userspace is running a command, then the kernel comes in
>and wants to run its own bg command, it will simply abort *anything* ongoing as a
>last resort. And since we have no kernel-context about whatever is running at that
>point, is *think* it is safe to assume it was user-initiated.

Yes, this seems a reasonable assumption.

Currently user space doesn't have a way to initiate background commands
through primary mailbox as there is no provision to provide timeout value.

By filtering out user-initiated background commands which do not support abort,
we can potentially have a default timeout or allow user space to provide a custom timeout value. 
 
Overall, the approach to cancel current background operation when new bg operation
is initiated seems elegant. 
>
>diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>index 7b0fad7f6c4d..bf0742546ea8 100644
>--- a/drivers/cxl/pci.c
>+++ b/drivers/cxl/pci.c
>@@ -374,7 +374,7 @@ static bool cxl_try_to_cancel_background(struct cxl_mailbox *cxl_mbox)
>		mds->security.sanitize_active = false;
>
>		dev_dbg(cxlds->dev, "Sanitization operation aborted\n");
>-	} else {
>+	} else if (atomic_read_acquire(&cxl_mbox->poll_bgop)){
>		/*
>		 * Kick the poller and wait for it to be done - no one else
>		 * can touch mbox regs. rcuwait_wake_up() provides full
>@@ -403,7 +403,9 @@ static int cxl_pci_mbox_send(struct cxl_mailbox *cxl_mbox,
>	 */
>	if (cxl_is_background_cmd(cmd->opcode)) {
>		if (mds->security.sanitize_active ||
>-		    atomic_read_acquire(&cxl_mbox->poll_bgop)) {
>+		    atomic_read_acquire(&cxl_mbox->poll_bgop) ||
>+		    /* userspace-initiated ? */
>+		    !cxl_mbox_background_done(cxlds)) {
>			if (!cxl_try_to_cancel_background(cxl_mbox)) {
>				mutex_unlock(&cxl_mbox->mbox_mutex);
>
--Ravi.