Fix broker disconnect to better handle SIGPIPE#548
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #548
Scan targets checked: wolfmqtt-bugs, wolfmqtt-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| #ifdef MSG_NOSIGNAL | ||
| rc = (int)send(sock, buf, (size_t)buf_len, MSG_NOSIGNAL); | ||
| #else | ||
| rc = (int)send(sock, buf, (size_t)buf_len, 0); |
There was a problem hiding this comment.
🟠 [Medium] Fallback write path still allows SIGPIPE · Logic errors
BrokerPosix_Write falls back to plain send(..., 0) when neither MSG_NOSIGNAL nor SO_NOSIGPIPE is available, so public POSIX-backend users outside wolfmqtt_broker() still take the original SIGPIPE termination path.
Fix: Add a fallback SIGPIPE suppression path or fail initialization when no per-send or per-socket suppression is available.
| * after PUBREC. The strong assertion is that ASan finds no UAF when we | ||
| * tear down the broker while the publisher is still in qos2_pending. */ | ||
| mc = &g_clients[1]; | ||
| mc->closed = 1; |
There was a problem hiding this comment.
🔵 [Low] Abrupt-close test never exercises abnormal close · Weak or missing assertions
mc->closed = 1 makes mock_read() return MQTT_CODE_ERROR_TIMEOUT, and BrokerClient_Process() treats that as idle, so this test never exercises the abnormal-close branch it is meant to cover.
Fix: Make the mock return MQTT_CODE_ERROR_NETWORK for this case and assert the publisher client is removed or closed.
| * publishes QoS>0 and immediately closes its socket would cause the | ||
| * broker's PUBACK/PUBREC write to deliver SIGPIPE, terminating the | ||
| * broker. Linux uses MSG_NOSIGNAL in BrokerPosix_Write instead. */ | ||
| (void)setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &on, sizeof(on)); |
There was a problem hiding this comment.
🔵 [Low] SIGPIPE suppression can silently remain disabled · Resource leaks
BrokerPosix_Accept ignores SO_NOSIGPIPE setup failure, so accepted sockets can still reach the zero-flag send() path and let a peer-closed write terminate the broker with SIGPIPE.
Fix: Check setsockopt and reject the accepted socket, or install a backend fallback before returning success.
3 participants
No description provided.