Re: [PATCH_v3 3/3] android/pan: Handle connection and control state notifications

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

 



Hi Johan,

On 11/13/2013 11:33 AM, Johan Hedberg wrote:
Hi Ravi,

On Wed, Nov 13, 2013, Ravi kumar Veeramally wrote:
---
  android/hal-pan.c | 31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)
I've applied the first two patches, but for this one I've got a few
questions:

diff --git a/android/hal-pan.c b/android/hal-pan.c
index 851c5d2..2bc560e 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,10 +31,41 @@ static bool interface_ready(void)
  	return cbs != NULL;
  }
+static void handle_conn_state(void *buf)
+{
+	struct hal_ev_pan_conn_state *ev = buf;
+
+	if (cbs->connection_state_cb)
+		cbs->connection_state_cb(ev->state, ev->status,
+					(bt_bdaddr_t *) ev->bdaddr,
+					ev->local_role, ev->remote_role);
+}
+
+static void handle_ctrl_state(void *buf)
+{
+	struct hal_ev_pan_ctrl_state *ev = buf;
+
+	if (cbs->control_state_cb)
+		cbs->control_state_cb(ev->state, ev->status,
+					ev->local_role, (char *)ev->name);
+}
+
  void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len)
  {
  	if (!interface_ready())
  		return;
+
+	switch (opcode) {
+	case HAL_EV_PAN_CONN_STATE:
+		handle_conn_state(buf);
+		break;
+	case HAL_EV_PAN_CTRL_STATE:
+		handle_ctrl_state(buf);
+		break;
+	default:
+		DBG("Unhandled callback opcode=0x%x", opcode);
+		break;
+	}
  }
What I don't like about this is that you're not pushing the data length
to the handler functions. If you did that the handler functions could:

	if (len < sizeof(*ev))
		return;

Instead of return we could also just abort - what's the general policy
on the HAL side regarding invalid data from the daemon? How does this
relate to the work Szymon is doing to add proper checks for the IPC
data? Is that only for the daemon side?

Are we missing similar checks in other HALs too?

Yes, we are not doing similar checks in other HALs (hal-bluetooth/a2dp/hid/) too. Very few places in hal-bluetooth length is passing but validation is not done.
  I will fix them all and send you the patch.

 Thanks,
 Ravi.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux