[ This code is a bit old. I don't know why it's only complaining now, a
year later. - dan ]
Hello Bjorn Andersson,
The patch 9b8a11e82615: "soc: qcom: Introduce QMI encoder/decoder"
from Dec 5, 2017, leads to the following static checker warning:
drivers/soc/qcom/qmi_encdec.c:549 qmi_decode_string_elem()
warn: array off by one? '*(buf_dst + string_len)'
drivers/soc/qcom/qmi_encdec.c
517 static int qmi_decode_string_elem(struct qmi_elem_info *ei_array,
518 void *buf_dst, const void *buf_src,
519 u32 tlv_len, int dec_level)
520 {
521 int rc;
522 int decoded_bytes = 0;
523 u32 string_len = 0;
524 u32 string_len_sz = 0;
525 struct qmi_elem_info *temp_ei = ei_array;
526
527 if (dec_level == 1) {
528 string_len = tlv_len;
529 } else {
530 string_len_sz = temp_ei->elem_len <= U8_MAX ?
531 sizeof(u8) : sizeof(u16);
532 rc = qmi_decode_basic_elem(&string_len, buf_src,
533 1, string_len_sz);
534 decoded_bytes += rc;
535 }
536
537 if (string_len > temp_ei->elem_len) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
538 pr_err("%s: String len %d > Max Len %d\n",
539 __func__, string_len, temp_ei->elem_len);
540 return -ETOOSMALL;
541 } else if (string_len > tlv_len) {
^^^^^^^^^^^^^^^^^^^^
542 pr_err("%s: String len %d > Input Buffer Len %d\n",
543 __func__, string_len, tlv_len);
544 return -EFAULT;
545 }
546
547 rc = qmi_decode_basic_elem(buf_dst, buf_src + decoded_bytes,
548 string_len, temp_ei->elem_size);
549 *((char *)buf_dst + string_len) = '\0';
^^^^^^^^^^^^^^^^^^^^
This is an unpublished Smatch test. How it works is it assume every
> length condition is off by one unless it can prove otherwise. Smatch
is surprisingly good at proving otherwise so it has maybe a 70% false
possitive in new code (100% false positive in old code because we fix
them). I have looked at this and I think it probably is a bug. The
buffer and length sometimes originate from qmi_data_ready_work().
550 decoded_bytes += rc;
551
552 return decoded_bytes;
553 }
regards,
dan carpenter
