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