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
