input_thread: use flb_pipe_r in handle_input_event to fix crash on re…#11623
input_thread: use flb_pipe_r in handle_input_event to fix crash on re…#11623zshuang0316 wants to merge 1 commit intofluent:masterfrom
Conversation
Signed-off-by: zshuang0316 <zshuang0316@163.com>
zshuang0316
force-pushed
the
fix/input-thread-use-flb-pipe-read
branch
from
ce4a66f to
c59f654
Compare
📝 WalkthroughWalkthroughThe input-thread event handler in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/flb_input_thread.c`:
- Around line 62-65: The handlers currently call flb_pipe_r(fd, &val,
sizeof(val)) and only treat bytes == -1 (or <= 0) as errors, which allows 0
(peer closed) or short reads to proceed and corrupt the decoded bit-field; in
handle_input_event and handle_input_thread_event update the read handling to
verify that the returned bytes == sizeof(val) and treat any other return (0 or
short read or -1) as an error/closed peer path (call flb_pipe_error() or the
existing cleanup flow), and apply the same full-length validation to the other
occurrences of flb_pipe_r reading fixed-size protocol messages so you never
decode a partially-populated val.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| bytes = flb_pipe_r(fd, &val, sizeof(val)); | ||
| if (bytes == -1) { | ||
| flb_errno(); | ||
| flb_pipe_error(); | ||
| return -1; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate mk_event_channel_create implementation/usages =="
rg -n -C3 'mk_event_channel_create|socketpair|pipe\(|recv\(|send\('
echo
echo "== Locate fixed-size flb_pipe_r reads and their guards =="
rg -n -C4 'flb_pipe_r\([^;]*sizeof\((val|uint64_t)\)\)' src include
echo
echo "== Inspect guards around handle_input_event and handle_input_thread_event =="
rg -n -C6 'handle_input_event|handle_input_thread_event|bytes == -1|bytes <= 0|bytes != sizeof' src/flb_input_thread.cRepository: fluent/fluent-bit
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading src/flb_input_thread.c lines 50-80 ==="
sed -n '50,80p' src/flb_input_thread.c
echo ""
echo "=== Checking flb_pipe.h for flb_pipe_r definition ==="
rg -n "^[^/]*flb_pipe_r" include/fluent-bit/flb_pipe.h -A 5
echo ""
echo "=== Looking for all guard patterns after flb_pipe_r calls ==="
rg -n -A 2 'flb_pipe_r.*sizeof' src/flb_input_thread.c
echo ""
echo "=== Checking both handler functions in detail ==="
rg -n -B 5 -A 8 'handle_input_event|handle_input_thread_event' src/flb_input_thread.c | head -80Repository: fluent/fluent-bit
Length of output: 3962
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for all fixed-size flb_pipe_r reads ==="
rg -n 'flb_pipe_r.*sizeof.*uint64_t|flb_pipe_r.*sizeof.*output_flush' src/ -A 3 | head -60
echo ""
echo "=== Check if short-read validation exists anywhere for fixed-size protocol messages ==="
rg -n 'bytes != sizeof|ret != sizeof' src/flb_input_thread.c src/flb_output_thread.c
echo ""
echo "=== Verify the exact bit-field macros being used ==="
rg -n 'FLB_BITS_U64_HIGH|FLB_BITS_U64_LOW' include/fluent-bit/ -B 1 -A 1 | head -30Repository: fluent/fluent-bit
Length of output: 1303
🏁 Script executed:
#!/bin/bash
set -euo pusedo pipefail
echo "=== Check mk_event_channel_create to understand channel semantics ==="
rg -n 'mk_event_channel_create' lib/monkey/include/monkey/ -A 15 2>/dev/null | head -50
echo ""
echo "=== Search for short-read handling in flb_pipe_write_all and flb_pipe_read_all ==="
cat src/flb_pipe.c 2>/dev/null || echo "File may not exist or other error"Repository: fluent/fluent-bit
Length of output: 112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for flb_pipe_read_all and flb_pipe_write_all implementations ==="
cat src/flb_pipe.c
echo ""
echo "=== Check if there's any documentation on atomicity ==="
rg -n 'socketpair|mk_event_channel' include/fluent-bit/ -B 2 -A 5 | head -60Repository: fluent/fluent-bit
Length of output: 5485
Check guards before decoding fixed-size protocol messages in event handlers.
Lines 63 and 107 only reject -1. With flb_pipe_r() (wrapping recv/read), returning 0 (closed peer) or short positive reads is possible. Without validating full message reception, partial val data will corrupt bit-field extraction and misroute events.
Proposed fix
bytes = flb_pipe_r(fd, &val, sizeof(val));
- if (bytes == -1) {
+ if (bytes <= 0) {
flb_pipe_error();
return -1;
}
+ if (bytes != sizeof(val)) {
+ flb_error("[thread event loop] short read on fd=%i, got=%i expected=%zu",
+ fd, bytes, sizeof(val));
+ return -1;
+ }Similar patterns exist at lines 437–438 and 774–775 in the same file, though they use only <= 0 checks without validating message completeness. Both handle_input_event and handle_input_thread_event need this fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/flb_input_thread.c` around lines 62 - 65, The handlers currently call
flb_pipe_r(fd, &val, sizeof(val)) and only treat bytes == -1 (or <= 0) as
errors, which allows 0 (peer closed) or short reads to proceed and corrupt the
decoded bit-field; in handle_input_event and handle_input_thread_event update
the read handling to verify that the returned bytes == sizeof(val) and treat any
other return (0 or short read or -1) as an error/closed peer path (call
flb_pipe_error() or the existing cleanup flow), and apply the same full-length
validation to the other occurrences of flb_pipe_r reading fixed-size protocol
messages so you never decode a partially-populated val.
1 participant
Issue
In
handle_input_event(), the raw POSIXread()syscall was used to read from the pipe file descriptor:On Windows,
read()is not compatible with pipe file descriptors created by Fluent Bit'sflb_pipeabstraction layer, which can cause a crash or undefined behavior. Even on Linux, bypassing the abstraction is inconsistent with the rest of the codebase.Solution
Replace the
raw read()call with the cross-platformflb_pipe_r()abstraction, and replaceflb_errno()withflb_pipe_error()for consistent error reporting — matching the pattern already used inhandle_input_thread_event()and elsewhere in the same file:Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Crashes disappeared after this change when enabled threaded in my test environment.
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit