Re: [PATCH v3 2/5] soc: qcom: Introduce QMI helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/15/2017 12:10 PM, Bjorn Andersson wrote:
[..]> +static void qmi_handle_message(struct qmi_handle *qmi,
+			       struct sockaddr_qrtr *sq,
+			       const void *buf, size_t len)
+{
+	const struct qmi_header *hdr;
+	struct qmi_txn tmp_txn;
+	struct qmi_txn *txn = NULL;
+	int ret;
+
+	if (len < sizeof(*hdr)) {
+		pr_err("ignoring short QMI packet\n");
+		return;
+	}
+
+	hdr = buf;
+
+	/* If this is a response, find the matching transaction handle */
+	if (hdr->type == QMI_RESPONSE) {
+		mutex_lock(&qmi->txn_lock);
+		txn = idr_find(&qmi->txns, hdr->txn_id);
+		if (txn)
+			mutex_lock(&txn->lock);
+		mutex_unlock(&qmi->txn_lock);
+	}
+
+	if (txn && txn->dest && txn->ei) {
+		ret = qmi_decode_message(buf, len, txn->ei, txn->dest);
+		if (ret < 0)
+			pr_err("failed to decode incoming message\n");
+
+		txn->result = ret;
+		complete(&txn->completion);
+
+		mutex_unlock(&txn->lock);
+	} else if (txn) {
+		qmi_invoke_handler(qmi, sq, txn, buf, len);
+
+		mutex_unlock(&txn->lock);
+	} else {
+		/* Create a txn based on the txn_id of the incoming message */
+		memset(&tmp_txn, 0, sizeof(tmp_txn));
+		tmp_txn.id = hdr->txn_id;
+
+		qmi_invoke_handler(qmi, sq, &tmp_txn, buf, len);

I'm seeing an opportunity for user error with timed out transactions. If txn_wait gets timed out the txn is removed from the txns list. Later if we get the response, it comes down to this else with the tmp_txn. Some handlers may try to do a complete(&txn->completion) like the qmi_sample_client ping_pong_cb. This leads to a null pointer dereference since the temp txn was never initialized.

Should clients not call complete and we can call the complete in the else if(txn) condition?

Thanks,
Chris
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux