cxl/mbox: Use type __u32 for mailbox payload sizes

Payload sizes for mailbox commands are expected to be positive values
coming from userspace. The documentation correctly describes these as
always unsigned values. The mailbox and send structures that support
the mailbox commands however, use __s32 types for the payloads.

Replace  __s32 with __u32 in the mailbox and send command structures
and update usages.

Kernel users of the interface already block all negative values and
there is no known ability for userspace to have grown a dependency on
submitting negative values to the kernel. The known user of the IOCTL,
the CXL command line interface (cxl-cli) already enforces positive
size values.

A Smatch warning of a signedness uncovered this issue.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Link: https://lore.kernel.org/r/20220414051246.1244575-1-alison.schofield@intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 8a83885..d54a6d1 100644 (file)
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -35,6 +35,7 @@ static bool cxl_raw_allow_all;
        .flags = _flags,                                                       \
        }
 
+#define CXL_VARIABLE_PAYLOAD   ~0U
 /*
  * This table defines the supported mailbox commands for the driver. This table
  * is made up of a UAPI structure. Non-negative values as parameters in the
@@ -44,26 +45,26 @@ static bool cxl_raw_allow_all;
 static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
        CXL_CMD(IDENTIFY, 0, 0x43, CXL_CMD_FLAG_FORCE_ENABLE),
 #ifdef CONFIG_CXL_MEM_RAW_COMMANDS
-       CXL_CMD(RAW, ~0, ~0, 0),
+       CXL_CMD(RAW, CXL_VARIABLE_PAYLOAD, CXL_VARIABLE_PAYLOAD, 0),
 #endif
-       CXL_CMD(GET_SUPPORTED_LOGS, 0, ~0, CXL_CMD_FLAG_FORCE_ENABLE),
+       CXL_CMD(GET_SUPPORTED_LOGS, 0, CXL_VARIABLE_PAYLOAD, CXL_CMD_FLAG_FORCE_ENABLE),
        CXL_CMD(GET_FW_INFO, 0, 0x50, 0),
        CXL_CMD(GET_PARTITION_INFO, 0, 0x20, 0),
-       CXL_CMD(GET_LSA, 0x8, ~0, 0),
+       CXL_CMD(GET_LSA, 0x8, CXL_VARIABLE_PAYLOAD, 0),
        CXL_CMD(GET_HEALTH_INFO, 0, 0x12, 0),
-       CXL_CMD(GET_LOG, 0x18, ~0, CXL_CMD_FLAG_FORCE_ENABLE),
+       CXL_CMD(GET_LOG, 0x18, CXL_VARIABLE_PAYLOAD, CXL_CMD_FLAG_FORCE_ENABLE),
        CXL_CMD(SET_PARTITION_INFO, 0x0a, 0, 0),
-       CXL_CMD(SET_LSA, ~0, 0, 0),
+       CXL_CMD(SET_LSA, CXL_VARIABLE_PAYLOAD, 0, 0),
        CXL_CMD(GET_ALERT_CONFIG, 0, 0x10, 0),
        CXL_CMD(SET_ALERT_CONFIG, 0xc, 0, 0),
        CXL_CMD(GET_SHUTDOWN_STATE, 0, 0x1, 0),
        CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0),
-       CXL_CMD(GET_POISON, 0x10, ~0, 0),
+       CXL_CMD(GET_POISON, 0x10, CXL_VARIABLE_PAYLOAD, 0),
        CXL_CMD(INJECT_POISON, 0x8, 0, 0),
        CXL_CMD(CLEAR_POISON, 0x48, 0, 0),
        CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
        CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
-       CXL_CMD(GET_SCAN_MEDIA, 0, ~0, 0),
+       CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
 };
 
 /*
@@ -187,9 +188,10 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
         * Variable sized commands can't be validated and so it's up to the
         * caller to do that if they wish.
         */
-       if (cmd->info.size_out >= 0 && mbox_cmd.size_out != out_size)
-               return -EIO;
-
+       if (cmd->info.size_out != CXL_VARIABLE_PAYLOAD) {
+               if (mbox_cmd.size_out != out_size)
+                       return -EIO;
+       }
        return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mbox_send_cmd, CXL);
@@ -275,7 +277,7 @@ static int cxl_mbox_cmd_ctor(struct cxl_mbox_cmd *mbox,
        }
 
        /* Prepare to handle a full payload for variable sized output */
-       if (out_size < 0)
+       if (out_size == CXL_VARIABLE_PAYLOAD)
                mbox->size_out = cxlds->payload_size;
        else
                mbox->size_out = out_size;
@@ -353,11 +355,11 @@ static int cxl_to_mem_cmd(struct cxl_mem_command *mem_cmd,
                return -EBUSY;
 
        /* Check the input buffer is the expected size */
-       if (info->size_in >= 0 && info->size_in != send_cmd->in.size)
+       if (info->size_in != send_cmd->in.size)
                return -ENOMEM;
 
        /* Check the output buffer is at least large enough */
-       if (info->size_out >= 0 && send_cmd->out.size < info->size_out)
+       if (send_cmd->out.size < info->size_out)
                return -ENOMEM;
 
        *mem_cmd = (struct cxl_mem_command) {

diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
index 8d206f2..c71021a 100644 (file)
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -68,8 +68,8 @@ static const struct {
  * struct cxl_command_info - Command information returned from a query.
  * @id: ID number for the command.
  * @flags: Flags that specify command behavior.
- * @size_in: Expected input size, or -1 if variable length.
- * @size_out: Expected output size, or -1 if variable length.
+ * @size_in: Expected input size, or ~0 if variable length.
+ * @size_out: Expected output size, or ~0 if variable length.
  *
  * Represents a single command that is supported by both the driver and the
  * hardware. This is returned as part of an array from the query ioctl. The
@@ -78,7 +78,7 @@ static const struct {
  *
  *  - @id = 10
  *  - @flags = 0
- *  - @size_in = -1
+ *  - @size_in = ~0
  *  - @size_out = 0
  *
  * See struct cxl_mem_query_commands.
@@ -89,8 +89,8 @@ struct cxl_command_info {
        __u32 flags;
 #define CXL_MEM_COMMAND_FLAG_MASK GENMASK(0, 0)
 
-       __s32 size_in;
-       __s32 size_out;
+       __u32 size_in;
+       __u32 size_out;
 };
 
 /**
@@ -169,13 +169,13 @@ struct cxl_send_command {
        __u32 retval;
 
        struct {
-               __s32 size;
+               __u32 size;
                __u32 rsvd;
                __u64 payload;
        } in;
 
        struct {
-               __s32 size;
+               __u32 size;
                __u32 rsvd;
                __u64 payload;
        } out;