linux-kernel - Re:[PATCH] soc: qmi: move tlv-micros to header file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <APkAOQB-CI1Ksc35Em0ifKrK.3.1589445179140.Hmail.wenhu.wang@vivo.com>
Date:   Thu, 14 May 2020 16:32:59 +0800 (GMT+08:00)
From:   王文虎 <wenhu.wang@...o.com>
To:     Wang Wenhu <wenhu.wang@...o.com>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Sibi Sankar <sibis@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...o.com
Subject: Re:[PATCH] soc: qmi: move tlv-micros to header file

Hi all,
Any comments here?

Regards,
Wenhu

>It's highly helpful to move the definitions of TLV related micros
>into header file for user reference. The OPTIONAL_TLV_TYPE_START
>should be known to any user that might define messages containing
>optional fields. SIZE fields are the same, for users to calculate
>message buffer length.
>
>While encoding messages, Type always occurs together with Length.
>So the better way is to use TL_SIZE, rather than (T_SIZE + L_SIZE).
>
>Signed-off-by: Wang Wenhu <wenhu.wang@...o.com>
>---
> drivers/soc/qcom/qmi_encdec.c | 28 ++++++++++++----------------
> include/linux/soc/qcom/qmi.h  |  5 +++++
> 2 files changed, 17 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
>index 3aaab71d1b2c..a674c68efab2 100644
>--- a/drivers/soc/qcom/qmi_encdec.c
>+++ b/drivers/soc/qcom/qmi_encdec.c
>@@ -53,10 +53,6 @@ do { \
> 	decoded_bytes += rc; \
> } while (0)
> 
>-#define TLV_LEN_SIZE sizeof(u16)
>-#define TLV_TYPE_SIZE sizeof(u8)
>-#define OPTIONAL_TLV_TYPE_START 0x10
>-
> static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
> 		      const void *in_c_struct, u32 out_buf_len,
> 		      int enc_level);
>@@ -142,7 +138,7 @@ static int qmi_calc_min_msg_len(struct qmi_elem_info *ei_array,
> 		 * nested structure.
> 		 */
> 		if (level == 1)
>-			min_msg_len += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
>+			min_msg_len += QMI_TLV_TL_SIZE;
> 	}
> 
> 	return min_msg_len;
>@@ -253,8 +249,7 @@ static int qmi_encode_string_elem(struct qmi_elem_info *ei_array,
> 	}
> 
> 	if (enc_level == 1) {
>-		if (string_len + TLV_LEN_SIZE + TLV_TYPE_SIZE >
>-		    out_buf_len) {
>+		if (string_len + QMI_TLV_TL_SIZE > out_buf_len) {
> 			pr_err("%s: Output len %d > Out Buf len %d\n",
> 			       __func__, string_len, out_buf_len);
> 			return -ETOOSMALL;
>@@ -311,7 +306,7 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
> 	tlv_pointer = buf_dst;
> 	tlv_len = 0;
> 	if (enc_level == 1)
>-		buf_dst = buf_dst + (TLV_LEN_SIZE + TLV_TYPE_SIZE);
>+		buf_dst = buf_dst + QMI_TLV_TL_SIZE;
> 
> 	while (temp_ei->data_type != QMI_EOTI) {
> 		buf_src = in_c_struct + temp_ei->offset;
>@@ -342,8 +337,8 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
> 			data_len_sz = temp_ei->elem_size == sizeof(u8) ?
> 					sizeof(u8) : sizeof(u16);
> 			/* Check to avoid out of range buffer access */
>-			if ((data_len_sz + encoded_bytes + TLV_LEN_SIZE +
>-			    TLV_TYPE_SIZE) > out_buf_len) {
>+			if ((data_len_sz + encoded_bytes + QMI_TLV_TL_SIZE) >
>+			    out_buf_len) {
> 				pr_err("%s: Too Small Buffer @DATA_LEN\n",
> 				       __func__);
> 				return -ETOOSMALL;
>@@ -367,7 +362,7 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
> 		case QMI_SIGNED_4_BYTE_ENUM:
> 			/* Check to avoid out of range buffer access */
> 			if (((data_len_value * temp_ei->elem_size) +
>-			    encoded_bytes + TLV_LEN_SIZE + TLV_TYPE_SIZE) >
>+			    encoded_bytes + QMI_TLV_TL_SIZE) >
> 			    out_buf_len) {
> 				pr_err("%s: Too Small Buffer @data_type:%d\n",
> 				       __func__, temp_ei->data_type);
>@@ -410,10 +405,10 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
> 
> 		if (encode_tlv && enc_level == 1) {
> 			QMI_ENCDEC_ENCODE_TLV(tlv_type, tlv_len, tlv_pointer);
>-			encoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
>+			encoded_bytes += QMI_TLV_TL_SIZE;
> 			tlv_pointer = buf_dst;
> 			tlv_len = 0;
>-			buf_dst = buf_dst + TLV_LEN_SIZE + TLV_TYPE_SIZE;
>+			buf_dst = buf_dst + QMI_TLV_TL_SIZE;
> 			encode_tlv = 0;
> 		}
> 	}
>@@ -613,10 +608,11 @@ static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
> 			tlv_pointer = buf_src;
> 			QMI_ENCDEC_DECODE_TLV(&tlv_type,
> 					      &tlv_len, tlv_pointer);
>-			buf_src += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
>-			decoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
>+			buf_src += QMI_TLV_TL_SIZE;
>+			decoded_bytes += QMI_TLV_TL_SIZE;
> 			temp_ei = find_ei(ei_array, tlv_type);
>-			if (!temp_ei && tlv_type < OPTIONAL_TLV_TYPE_START) {
>+			if (!temp_ei && tlv_type <
>+			    QMI_OPTIONAL_TLV_TYPE_START) {
> 				pr_err("%s: Inval element info\n", __func__);
> 				return -EINVAL;
> 			} else if (!temp_ei) {
>diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
>index e712f94b89fc..eb53aebdf45e 100644
>--- a/include/linux/soc/qcom/qmi.h
>+++ b/include/linux/soc/qcom/qmi.h
>@@ -34,6 +34,11 @@ struct qmi_header {
> #define QMI_INDICATION	4
> 
> #define QMI_COMMON_TLV_TYPE 0
>+#define QMI_OPTIONAL_TLV_TYPE_START 0x10
>+
>+#define QMI_TLV_LEN_SIZE sizeof(u16)
>+#define QMI_TLV_TYPE_SIZE sizeof(u8)
>+#define QMI_TLV_TL_SIZE (QMI_TLV_LEN_SIZE + QMI_TLV_TYPE_SIZE)
> 
> enum qmi_elem_type {
> 	QMI_EOTI,
>-- 
>2.17.1
>