[bug report] misc: amd-sbi: Add support for CPUID protocol

[Dan Carpenter <dan.carpenter@linaro.org>]

Hello Akshay Gupta,

Commit bb13a84ed6b7 ("misc: amd-sbi: Add support for CPUID protocol")
from Apr 28, 2025 (linux-next), leads to the following Smatch static
checker warning:

drivers/misc/amd-sbi/rmi-core.c:132 rmi_cpuid_read() warn: bitwise OR is zero '0xffffffff00000000 & 0xffff'
drivers/misc/amd-sbi/rmi-core.c:132 rmi_cpuid_read() warn: potential integer overflow from user 'msg->cpu_in_out << 32'
drivers/misc/amd-sbi/rmi-core.c:213 rmi_mca_msr_read() warn: bitwise OR is zero '0xffffffff00000000 & 0xffff'
drivers/misc/amd-sbi/rmi-core.c:213 rmi_mca_msr_read() warn: potential integer overflow from user 'msg->mcamsr_in_out << 32'
drivers/misc/amd-sbi/rmi-core.c:376 apml_rmi_reg_xfer() warn: maybe return -EFAULT instead of the bytes remaining?
drivers/misc/amd-sbi/rmi-core.c:394 apml_mailbox_xfer() warn: maybe return -EFAULT instead of the bytes remaining?
drivers/misc/amd-sbi/rmi-core.c:411 apml_cpuid_xfer() warn: maybe return -EFAULT instead of the bytes remaining?
drivers/misc/amd-sbi/rmi-core.c:428 apml_mcamsr_xfer() warn: maybe return -EFAULT instead of the bytes remaining?

drivers/misc/amd-sbi/rmi-core.c
   110  static int rmi_cpuid_read(struct sbrmi_data *data,
   111                            struct apml_cpuid_msg *msg)
   112  {
   113          struct cpu_msr_indata input = {0};
   114          struct cpu_msr_outdata output = {0};
   115          int val = 0;
   116          int ret, hw_status;
   117          u16 thread;
   118  
   119          mutex_lock(&data->lock);
   120          /* cache the rev value to identify if protocol is supported or not */
   121          if (!data->rev) {
   122                  ret = sbrmi_get_rev(data);
   123                  if (ret < 0)
   124                          goto exit_unlock;
   125          }
   126          /* CPUID protocol for REV 0x10 is not supported*/
   127          if (data->rev == 0x10) {
   128                  ret = -EOPNOTSUPP;
   129                  goto exit_unlock;
   130          }
   131  
   132          thread = msg->cpu_in_out << CPUID_MCA_THRD_INDEX & CPUID_MCA_THRD_MASK;

CPUID_MCA_THRD_INDEX is 32.
CPUID_MCA_THRD_MASK is 0xffff.

Smatch complains that msg->cpu_in_out is user data but we're shifting
away some bits which is suspicious (but can be fine).  But then the
result of the shift mask we save in thread is always zero.

   133  
   134          /* Thread > 127, Thread128 CS register, 1'b1 needs to be set to 1 */
   135          if (thread > 127) {
   136                  thread -= 128;
   137                  val = 1;
   138          }
   139          ret = regmap_write(data->regmap, SBRMI_THREAD128CS, val);
   140          if (ret < 0)
   141                  goto exit_unlock;
   142  
   143          prepare_cpuid_input_message(&input, thread,
   144                                      msg->cpu_in_out & CPUID_MCA_FUNC_MASK,

Maybe it was suppoesd to be just "& CPUID_MCA_FUNC_MASK" without the
shift?

   145                                      msg->cpu_in_out >> CPUID_EXT_FUNC_INDEX);
   146  
   147          ret = regmap_bulk_write(data->regmap, CPUID_MCA_CMD,
   148                                  &input, CPUID_WR_REG_LEN);
   149          if (ret < 0)


[ snip ]

   353  static int apml_rmi_reg_xfer(struct sbrmi_data *data,
   354                               struct apml_reg_xfer_msg __user *arg)
   355  {
   356          struct apml_reg_xfer_msg msg = { 0 };
   357          unsigned int data_read;
   358          int ret;
   359  
   360          /* Copy the structure from user */
   361          if (copy_from_user(&msg, arg, sizeof(struct apml_reg_xfer_msg)))
   362                  return -EFAULT;
   363  
   364          mutex_lock(&data->lock);
   365          if (msg.rflag) {
   366                  ret = regmap_read(data->regmap, msg.reg_addr, &data_read);
   367                  if (!ret)
   368                          msg.data_in_out = data_read;
   369          } else {
   370                  ret = regmap_write(data->regmap, msg.reg_addr, msg.data_in_out);
   371          }
   372  
   373          mutex_unlock(&data->lock);
   374  
   375          if (msg.rflag && !ret)
   376                  return copy_to_user(arg, &msg, sizeof(struct apml_reg_xfer_msg));
                               ^^^^^^^^^^^^
copy_to/from_user() returns the number of bytes that it wasn't able to
copy.  This should be:

	if (ret)
		return ret;

	if (msg.rflag) {
		if (copy_to_user(arg, &msg, sizeof(struct apml_reg_xfer_msg)))
			return -EFAULT;
	}

	return 0;

   377          return ret;
   378  }

regards,
dan carpenter