Bluetooth: Controller: Fix missing conn update ind PDU validation

[cvinayak]

Fix missing validation of Connection Update Ind PDU. Ignore invalid connection update parameters and let the connection either procedure timeout or supervision timeout if the central switched to using invalid connection parameters.

[cvinayak]

@thoh-ot @erbr-ot @wopu-ot @kruithofa is this behavior to let procedure/supervision timeout an acceptable outcome?
Will have to add/update unit tests accordingly.

[erbr-ot]

I tend to think we should be more pro-active in case of invalid params and disconnect with BT_HCI_ERR_INVALID_LL_PARAM. This could make recovery from a central mistake faster.

[thoh-ot]

Take at look at how invalid LL_PHY_UPDATE_IND are handled in

zephyr/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c

Lines 714 to 747 in 99adbad

	static void lp_pu_st_wait_rx_phy_update_ind(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt,
	void *param)
	{
	switch (evt) {
	case LP_PU_EVT_PHY_UPDATE_IND:
	LL_ASSERT(conn->lll.role == BT_HCI_ROLE_PERIPHERAL);
	llcp_rr_set_incompat(conn, INCOMPAT_RESERVED);
	llcp_pdu_decode_phy_update_ind(ctx, (struct pdu_data *)param);
	const uint8_t end_procedure = pu_check_update_ind(conn, ctx);
	/* Mark RX node to NOT release */
	llcp_rx_node_retain(ctx);
	if (!end_procedure) {
	if (ctx->data.pu.p_to_c_phy) {
	/* If periph to central phy changes apply tx timing restriction */
	pu_set_timing_restrict(conn, ctx->data.pu.p_to_c_phy);
	}
	/* Since at least one phy will change,
	* stop the procedure response timeout
	*/
	llcp_lr_prt_stop(conn);
	ctx->state = LP_PU_STATE_WAIT_INSTANT;
	} else {
	llcp_rr_set_incompat(conn, INCOMPAT_NO_COLLISION);
	if (ctx->data.pu.error != BT_HCI_ERR_SUCCESS) {
	/* Mark the connection for termination */
	conn->llcp_terminate.reason_final = ctx->data.pu.error;
	}
	ctx->data.pu.ntf_pu = ctx->data.pu.host_initiated;
	lp_pu_complete(conn, ctx, evt, param);
	}

[Thalley]

Changes look good, with some minor suggestions for improved readability

[Thalley]

Suggest to make this more readable by using the IN_RANGE macro

[Thalley]

Suggest to rename check to valid or something.

If a check function returns true, is it really obvious that it's valid?

IMO check works better with int than bool