Thread (48 messages) 48 messages, 8 authors, 2024-08-09

Re: [PATCH v2 5/8] fwctl: FWCTL_RPC to execute a Remote Procedure Call to device firmware

From: Leon Romanovsky <leon@kernel.org>
Date: 2024-07-30 08:00:44
Also in: linux-cxl, linux-doc, linux-patches, linux-rdma

On Mon, Jun 24, 2024 at 07:47:29PM -0300, Jason Gunthorpe wrote:
quoted hunk ↗ jump to hunk
Add the FWCTL_RPC ioctl which allows a request/response RPC call to device
firmware. Drivers implementing this call must follow the security
guidelines under Documentation/userspace-api/fwctl.rst

The core code provides some memory management helpers to get the messages
copied from and back to userspace. The driver is responsible for
allocating the output message memory and delivering the message to the
device.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/fwctl/main.c       | 62 +++++++++++++++++++++++++++++++++++
 include/linux/fwctl.h      |  5 +++
 include/uapi/fwctl/fwctl.h | 66 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
diff --git a/drivers/fwctl/main.c b/drivers/fwctl/main.c
index f1dec0b590aee4..9506b993a1a56d 100644
--- a/drivers/fwctl/main.c
+++ b/drivers/fwctl/main.c
@@ -8,16 +8,20 @@
 #include <linux/slab.h>
 #include <linux/container_of.h>
 #include <linux/fs.h>
+#include <linux/sizes.h>
 
 #include <uapi/fwctl/fwctl.h>
 
 enum {
 	FWCTL_MAX_DEVICES = 256,
+	MAX_RPC_LEN = SZ_2M,
 };
 static dev_t fwctl_dev;
 static DEFINE_IDA(fwctl_ida);
+static unsigned long fwctl_tainted;
 
 DEFINE_FREE(kfree_errptr, void *, if (!IS_ERR_OR_NULL(_T)) kfree(_T));
+DEFINE_FREE(kvfree_errptr, void *, if (!IS_ERR_OR_NULL(_T)) kvfree(_T));
 
 struct fwctl_ucmd {
 	struct fwctl_uctx *uctx;
@@ -75,9 +79,66 @@ static int fwctl_cmd_info(struct fwctl_ucmd *ucmd)
 	return ucmd_respond(ucmd, sizeof(*cmd));
 }
 
+static int fwctl_cmd_rpc(struct fwctl_ucmd *ucmd)
+{
+	struct fwctl_device *fwctl = ucmd->uctx->fwctl;
+	struct fwctl_rpc *cmd = ucmd->cmd;
+	size_t out_len;
+
+	if (cmd->in_len > MAX_RPC_LEN || cmd->out_len > MAX_RPC_LEN)
+		return -EMSGSIZE;
+
+	switch (cmd->scope) {
+	case FWCTL_RPC_CONFIGURATION:
+	case FWCTL_RPC_DEBUG_READ_ONLY:
+		break;
+
+	case FWCTL_RPC_DEBUG_WRITE_FULL:
+		if (!capable(CAP_SYS_RAWIO))
+			return -EPERM;
+		fallthrough;
+	case FWCTL_RPC_DEBUG_WRITE:
+		if (!test_and_set_bit(0, &fwctl_tainted)) {
+			dev_warn(
+				&fwctl->dev,
+				"%s(%d): has requested full access to the physical device device",
+				current->comm, task_pid_nr(current));
+			add_taint(TAINT_FWCTL, LOCKDEP_STILL_OK);
+		}
+		break;
+	default:
+		return -EOPNOTSUPP;
+	};
+
+	void *inbuf __free(kvfree) =
+		kvzalloc(cmd->in_len, GFP_KERNEL | GFP_KERNEL_ACCOUNT);

<...>
+	out_len = cmd->out_len;
+	void *outbuf __free(kvfree_errptr) = fwctl->ops->fw_rpc(
+		ucmd->uctx, cmd->scope, inbuf, cmd->in_len, &out_len);
I was under impression that declaration of variables in C should be at the beginning
of block. Was it changed for the kernel?

Thanks
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help