From 03d011e0396de6543146c8d0ccd16346d090754e Mon Sep 17 00:00:00 2001
From: Ulf Lilleengen <ulf.lilleengen@gmail.com>
Date: Mon, 12 Aug 2024 08:39:06 +0200
Subject: [PATCH 1/3] Improve ACL processing error handling

Ensure links are not kept lingering on all errors.
---
 host/src/host.rs | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/host/src/host.rs b/host/src/host.rs
index 4031277a..10457d79 100644
--- a/host/src/host.rs
+++ b/host/src/host.rs
@@ -899,6 +899,7 @@ where
                 Ok(_) => {}
                 Err(e) => {
                     warn!("Error dispatching l2cap packet to channel: {:?}", e);
+                    return Err(e);
                 }
             },
             _ => {
@@ -1054,14 +1055,33 @@ where
                     Ok(ControllerToHostPacket::Acl(acl)) => match self.handle_acl(acl) {
                         Ok(_) => {}
                         Err(e) => {
-                            trace!("Error processing ACL packet: {:?}", e);
-                            if let Error::OutOfMemory = e {
-                                warn!("[host] out of memory error on handle {:?}, disconnecting", acl.handle());
-                                self.connections.request_handle_disconnect(
-                                    acl.handle(),
-                                    DisconnectReason::RemoteDeviceTerminatedConnLowResources,
-                                );
-                            }
+                            // We disconnect on errors to ensure we don't leave the other end thinking
+                            // everything is ok.
+                            let reason = match e {
+                                Error::OutOfMemory => {
+                                    // Disconnect link due to low resources.
+                                    warn!("[host] out of memory error when processing ACL packet");
+                                    DisconnectReason::RemoteDeviceTerminatedConnLowResources
+                                }
+                                Error::Disconnected => {
+                                    // Already disconnected, request a disconnect to ensure we don't have
+                                    // any lingering connections, should be a noop
+                                    warn!("[host] already disconnected when processing ACL packet");
+                                    DisconnectReason::RemoteUserTerminatedConn
+                                }
+                                Error::NotSupported => {
+                                    // Attempt to use a feature not supported
+                                    warn!("[host] attempted to use unsupported feature when processing ACL packet");
+                                    DisconnectReason::UnsupportedRemoteFeature
+                                }
+                                e => {
+                                    // Otherwise blame the user.
+                                    warn!("[host] encountered error processing ACL packet: {:?}", e);
+                                    DisconnectReason::RemoteUserTerminatedConn
+                                }
+                            };
+                            warn!("[host] disconnecting handle {:?} after error", acl.handle());
+                            self.connections.request_handle_disconnect(acl.handle(), reason);
                             let mut m = self.metrics.borrow_mut();
                             m.rx_errors = m.rx_errors.wrapping_add(1);
                         }

From f00bff9180c48466f40931861cc6b4978aa8e1a9 Mon Sep 17 00:00:00 2001
From: Ulf Lilleengen <ulf.lilleengen@gmail.com>
Date: Mon, 12 Aug 2024 08:48:06 +0200
Subject: [PATCH 2/3] add 'log' feature to integration tests

---
 .github/workflows/tests.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
index d41f1e52..d9218587 100644
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@@ -34,7 +34,7 @@ jobs:
           RUST_LOG: trace
         run: |
           cd host
-          cargo test --test '*' -- --nocapture
+          cargo test --features log --test '*' -- --nocapture
       - name: Update failed status
         if: failure()
         env:

From 9a3818ef3d975969a4b3e7e385251a998fc1c258 Mon Sep 17 00:00:00 2001
From: Ulf Lilleengen <ulf.lilleengen@gmail.com>
Date: Mon, 12 Aug 2024 08:55:33 +0200
Subject: [PATCH 3/3] more error handling

---
 host/src/channel_manager.rs | 3 +++
 host/src/host.rs            | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/host/src/channel_manager.rs b/host/src/channel_manager.rs
index e23900f6..c7df88f6 100644
--- a/host/src/channel_manager.rs
+++ b/host/src/channel_manager.rs
@@ -289,6 +289,9 @@ impl<'d, const RXQ: usize> ChannelManager<'d, RXQ> {
                 match storage.state {
                     ChannelState::Connected if header.channel == storage.cid => {
                         if storage.flow_control.available() == 0 {
+                            // NOTE: This will trigger closing of the link, which might be a bit
+                            // too strict. But it should be controllable via the credits given,
+                            // which the remote should respect.
                             trace!("[l2cap][cid = {}] no credits available", header.channel);
                             return Err(Error::OutOfMemory);
                         }
diff --git a/host/src/host.rs b/host/src/host.rs
index 10457d79..bc366c05 100644
--- a/host/src/host.rs
+++ b/host/src/host.rs
@@ -814,7 +814,7 @@ where
                     && !(&[L2CAP_CID_LE_U_SIGNAL, L2CAP_CID_ATT].contains(&header.channel))
                 {
                     warn!("[host] unsupported l2cap channel id {}", header.channel);
-                    return Ok(());
+                    return Err(Error::NotSupported);
                 }
 
                 // Avoids using the packet buffer for signalling packets
@@ -903,7 +903,7 @@ where
                 }
             },
             _ => {
-                unimplemented!()
+                return Err(Error::NotSupported);
             }
         }
         Ok(())