[bug report] firmware: arm_scmi: Add per-channel Raw injection support

Dan Carpenter
2023-01-17T10:44:55Z

Hello Cristian Marussi,

The patch 0ae95d6e8b75: "firmware: arm_scmi: Add per-channel Raw
injection support" from Jan 13, 2023, leads to the following Smatch
static checker warning:

drivers/firmware/arm_scmi/raw_mode.c:1115 scmi_raw_mode_setup()
warn: 'q' is an error pointer or valid

drivers/firmware/arm_scmi/raw_mode.c
1087 static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw,
1088 u8 *channels, int num_chans)
1089 {
1090 int ret, idx;
1091 void *gid;
1092 struct device *dev = raw->handle->dev;
1093
1094 gid = devres_open_group(dev, NULL, GFP_KERNEL);
1095 if (!gid)
1096 return -ENOMEM;
1097
1098 for (idx = 0; idx < SCMI_RAW_MAX_QUEUE; idx++) {
1099 raw->q[idx] = scmi_raw_queue_init(raw);
1100 if (IS_ERR(raw->q[idx])) {
1101 ret = PTR_ERR(raw->q[idx]);
1102 goto err;
1103 }
1104 }
1105
1106 if (num_chans > 1) {
1107 int i;
1108
1109 idr_init(&raw->queues_idr);
1110
1111 for (i = 0; i < num_chans; i++) {
1112 struct scmi_raw_queue *q;
1113
1114 q = scmi_raw_queue_init(raw);
--> 1115 if (!q)
1116 continue;

This should be something like:

if (IS_ERR(q)) {
ret = PTR_ERR(q);
goto err;
}

1117
1118 ret = idr_alloc(&raw->queues_idr, q,
1119 channels[i], channels[i] + 1,
1120 GFP_KERNEL);
1121 if (ret != channels[i])
1122 dev_err(dev,
1123 "Fail to allocate Raw queue 0x%02X\n",
1124 channels[i]);

Heh. I've never seen anyone use an IDR range of one value before...
Just printing an error is not correct error handling. I think these
IDR values have to be freed on the error path? Or is there a devm_
trick happening here?

1125 }
1126 }
1127
1128 ret = scmi_xfer_raw_worker_init(raw);
1129 if (ret)
1130 goto err;
1131
1132 devres_close_group(dev, gid);
1133 raw->gid = gid;
1134
1135 return 0;
1136
1137 err:
1138 devres_release_group(dev, gid);
1139 return ret;
1140 }

regards,
dan carpenter