Skip to content

Commit 347ca17

Browse files
committed
Merge tag '2026-04-30.1' into push-2026-04-30.1
Change-Id: I0add21a13e6daeb7b270ca74dc44d5f897fe61f0
2 parents 0406ab8 + 572ad3f commit 347ca17

5 files changed

Lines changed: 238 additions & 10 deletions

File tree

src/pcm-sensor-server.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,16 +1224,20 @@ class basic_socketbuf : public std::basic_streambuf<CharT> {
12241224
}
12251225

12261226
virtual int_type overflow( int_type ch ) override {
1227-
// send data in buffer and reset it
1227+
// Flush buffer first - when overflow() is called, pptr() == epptr() (buffer is full)
1228+
// Writing to pptr() before flushing would write past the end of outputBuffer_
1229+
int_type bytesWritten = writeToSocket();
1230+
if ( traits_type::eof() == bytesWritten ) {
1231+
return traits_type::eof();
1232+
}
1233+
// Reset put area pointers to start of buffer after successful flush
1234+
Base::setp( outputBuffer_, outputBuffer_ + SIZE );
1235+
// Now safe to write new character at pptr() (start of empty buffer)
12281236
if ( traits_type::eof() != ch ) {
1229-
*Base::pptr() = ch;
1237+
*Base::pptr() = traits_type::to_char_type(ch);
12301238
Base::pbump(1);
12311239
}
1232-
int_type bytesWritten = 0;
1233-
if ( traits_type::eof() == (bytesWritten = writeToSocket()) ) {
1234-
return traits_type::eof();
1235-
}
1236-
return bytesWritten; // Anything but traits_type::eof() to signal ok.
1240+
return traits_type::not_eof( ch );
12371241
}
12381242

12391243
virtual int_type underflow() override {

src/utils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,7 @@ inline bool readOldValueHelper(const std::pair<int64,int64> & bits, T & value, c
702702
{
703703
return false;
704704
}
705-
value = insertBits(old_value, value, bits.first, bits.second - bits.first + 1);
705+
value = insertBits(old_value, value, bits.first, static_cast<uint64>(bits.second - bits.first + 1));
706706
}
707707
return true;
708708
}
@@ -715,7 +715,7 @@ inline void extractBitsPrintHelper(const std::pair<int64,int64> & bits, T & valu
715715
{
716716
std::cout << "bits "<< std::dec << bits.first << ":" << bits.second << " ";
717717
if (!dec) std::cout << std::hex << std::showbase;
718-
value = extract_bits(value, bits.first, bits.second);
718+
value = extract_bits(value, static_cast<uint32>(bits.first), static_cast<uint32>(bits.second));
719719
}
720720
std::cout << "value " << value;
721721
}

tests/test.sh

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,72 @@ if [ "$?" -ne "0" ]; then
220220
fi
221221

222222
# TODO add more tests
223-
# e.g for ./pcm-sensor-server, ./pcm-sensor, ...
223+
# e.g for ./pcm-sensor, ...
224+
225+
echo Testing pcm-sensor-server
226+
# Pick an unused high port to avoid collisions with the default one.
227+
SENSOR_PORT=19738
228+
./pcm-sensor-server -p $SENSOR_PORT -silent &
229+
SENSOR_PID=$!
230+
231+
# Wait for the server to start accepting connections (up to ~20s).
232+
SENSOR_READY=0
233+
for i in $(seq 1 40); do
234+
if ! kill -0 $SENSOR_PID 2>/dev/null; then
235+
echo "pcm-sensor-server exited unexpectedly during startup"
236+
exit 1
237+
fi
238+
if curl -s -o /dev/null -f "http://127.0.0.1:${SENSOR_PORT}/metrics"; then
239+
SENSOR_READY=1
240+
break
241+
fi
242+
sleep 0.5
243+
done
244+
245+
if [ "$SENSOR_READY" -ne "1" ]; then
246+
echo "Error: pcm-sensor-server did not become ready on port $SENSOR_PORT"
247+
kill $SENSOR_PID 2>/dev/null
248+
wait $SENSOR_PID 2>/dev/null
249+
exit 1
250+
fi
251+
252+
echo " Query /metrics endpoint with curl"
253+
METRICS_OUT=$(curl -s -f "http://127.0.0.1:${SENSOR_PORT}/metrics")
254+
CURL_RC=$?
255+
if [ "$CURL_RC" -ne "0" ] || [ -z "$METRICS_OUT" ]; then
256+
echo "Error in pcm-sensor-server: /metrics request failed or returned empty body"
257+
kill $SENSOR_PID 2>/dev/null
258+
wait $SENSOR_PID 2>/dev/null
259+
exit 1
260+
fi
261+
# Prometheus exposition format lines start with '#' (HELP/TYPE) or a metric name.
262+
if ! echo "$METRICS_OUT" | grep -q '^#'; then
263+
echo "Error in pcm-sensor-server: /metrics response does not look like Prometheus output"
264+
kill $SENSOR_PID 2>/dev/null
265+
wait $SENSOR_PID 2>/dev/null
266+
exit 1
267+
fi
268+
269+
echo " Query /dashboard endpoint with curl"
270+
curl -s -f -o /dev/null "http://127.0.0.1:${SENSOR_PORT}/dashboard"
271+
if [ "$?" -ne "0" ]; then
272+
echo "Error in pcm-sensor-server: /dashboard request failed"
273+
kill $SENSOR_PID 2>/dev/null
274+
wait $SENSOR_PID 2>/dev/null
275+
exit 1
276+
fi
277+
278+
echo " Query unknown endpoint, expect HTTP 404"
279+
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" "http://127.0.0.1:${SENSOR_PORT}/does-not-exist")
280+
if [ "$HTTP_CODE" != "404" ]; then
281+
echo "Error in pcm-sensor-server: expected 404 for unknown endpoint, got $HTTP_CODE"
282+
kill $SENSOR_PID 2>/dev/null
283+
wait $SENSOR_PID 2>/dev/null
284+
exit 1
285+
fi
286+
287+
kill $SENSOR_PID 2>/dev/null
288+
wait $SENSOR_PID 2>/dev/null
224289

225290
echo Testing urltest
226291
./tests/urltest

tests/utests/CMakeLists.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ endif()
1717
file(GLOB LSPCI_TEST_FILES lspci-utest.cpp ${CMAKE_SOURCE_DIR}/src/lspci.cpp)
1818
file(GLOB PCM_IIO_TEST_FILES pcm-iio-utest.cpp ${CMAKE_SOURCE_DIR}/src/pcm-iio-pmu.cpp ${CMAKE_SOURCE_DIR}/src/pcm-iio-topology.cpp)
1919
file(GLOB READ_NUMBER_TEST_FILES read-number-utest.cpp)
20+
file(GLOB PCM_SENSOR_SERVER_OVERFLOW_TEST_FILES pcm-sensor-server-overflow-utest.cpp)
2021

2122
if(APPLE)
2223
set(LIBS PcmMsr Threads::Threads PCM_STATIC)
@@ -27,6 +28,7 @@ endif()
2728
add_executable(lspci-utest ${LSPCI_TEST_FILES})
2829
add_executable(pcm-iio-utest ${PCM_IIO_TEST_FILES})
2930
add_executable(read-number-utest ${READ_NUMBER_TEST_FILES})
31+
add_executable(pcm-sensor-server-overflow-utest ${PCM_SENSOR_SERVER_OVERFLOW_TEST_FILES})
3032

3133
configure_file(
3234
${CMAKE_SOURCE_DIR}/src/opCode-6-174.txt
@@ -55,7 +57,15 @@ target_link_libraries(
5557
${LIBS}
5658
)
5759

60+
target_link_libraries(
61+
pcm-sensor-server-overflow-utest
62+
GTest::gtest_main
63+
GTest::gmock_main
64+
${LIBS}
65+
)
66+
5867
include(GoogleTest)
5968
gtest_discover_tests(lspci-utest)
6069
gtest_discover_tests(pcm-iio-utest)
6170
gtest_discover_tests(read-number-utest)
71+
gtest_discover_tests(pcm-sensor-server-overflow-utest)
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
// Copyright (c) 2009-2025, Intel Corporation
3+
4+
// Regression test for the out-of-bounds write in
5+
// basic_socketbuf::overflow() (src/pcm-sensor-server.cpp, lines 1226-1237).
6+
//
7+
// The vulnerable overflow() writes the incoming character into *pptr()
8+
// *before* flushing the full put-area to the socket. When the put area is
9+
// full, pptr() == epptr(), i.e. it points one past the end of outputBuffer_,
10+
// which is the first byte of the adjacent inputBuffer_. This test drives the
11+
// real basic_socketbuf template through a socketpair, fills the put area to
12+
// the brim, triggers overflow(), and verifies that inputBuffer_[0] is not
13+
// corrupted.
14+
//
15+
// With the bug in place the assertion on inputBuffer_[0] fails (and, when the
16+
// binary is built with AddressSanitizer via -DPCM_NO_ASAN=OFF, the underlying
17+
// intra-object OOB write / OOB read in send() is also detected). Once
18+
// overflow() is fixed to flush first and then store the character into the
19+
// emptied buffer, the test passes.
20+
21+
#include <sys/socket.h>
22+
#include <sys/types.h>
23+
#include <unistd.h>
24+
#include <atomic>
25+
#include <cstring>
26+
#include <memory>
27+
#include <thread>
28+
#include <vector>
29+
30+
// Pull the real basic_socketbuf template out of pcm-sensor-server.cpp without
31+
// bringing in its main(). The same mechanism is already used by
32+
// tests/pcm-sensor-server-fuzz.cpp.
33+
#define UNIT_TEST 1
34+
#include "../../src/pcm-sensor-server.cpp"
35+
#undef UNIT_TEST
36+
37+
#include <gtest/gtest.h>
38+
39+
namespace {
40+
41+
// SIZE matches the instantiation used by basic_socketstream::buf_type
42+
// (see pcm-sensor-server.cpp line 1363).
43+
constexpr std::size_t kSocketBufSize = 16385;
44+
45+
// Subclass that exposes the protected buffer members so the test can inspect
46+
// inputBuffer_[0] after triggering overflow().
47+
class ProbeSocketBuf : public basic_socketbuf<kSocketBufSize, char> {
48+
public:
49+
using basic_socketbuf<kSocketBufSize, char>::inputBuffer_;
50+
using basic_socketbuf<kSocketBufSize, char>::outputBuffer_;
51+
};
52+
53+
// Drain the peer end of a socketpair in a background thread so that the
54+
// server-side send() inside writeToSocket() never blocks, regardless of what
55+
// the kernel's default socket buffer sizes happen to be on the CI runner.
56+
class SocketDrainer {
57+
SocketDrainer & operator = (const SocketDrainer &) = delete;
58+
SocketDrainer(const SocketDrainer &) = delete;
59+
public:
60+
explicit SocketDrainer(int fd) : fd_(fd), stop_(false) {
61+
thread_ = std::thread([this]() {
62+
char buf[4096];
63+
while (!stop_.load()) {
64+
ssize_t n = ::recv(fd_, buf, sizeof(buf), 0);
65+
if (n <= 0) {
66+
break;
67+
}
68+
received_.insert(received_.end(), buf, buf + n);
69+
}
70+
});
71+
}
72+
73+
~SocketDrainer() {
74+
stop_.store(true);
75+
if (fd_ >= 0) {
76+
::shutdown(fd_, SHUT_RDWR);
77+
}
78+
if (thread_.joinable()) {
79+
thread_.join();
80+
}
81+
if (fd_ >= 0) {
82+
::close(fd_);
83+
fd_ = -1;
84+
}
85+
}
86+
87+
const std::vector<char>& data() const { return received_; }
88+
89+
private:
90+
int fd_;
91+
std::atomic<bool> stop_;
92+
std::thread thread_;
93+
std::vector<char> received_;
94+
};
95+
96+
} // namespace
97+
98+
TEST(PcmSensorServerOverflowTest, OverflowDoesNotWritePastOutputBuffer)
99+
{
100+
int sv[2];
101+
ASSERT_EQ(0, ::socketpair(AF_UNIX, SOCK_STREAM, 0, sv))
102+
<< "socketpair failed: " << std::strerror(errno);
103+
104+
// Peer drains whatever the socketbuf sends.
105+
SocketDrainer drainer(sv[1]);
106+
107+
auto buf = std::make_unique<ProbeSocketBuf>();
108+
buf->setSocket(sv[0]);
109+
110+
// Plant a distinctive sentinel in inputBuffer_[0]. With the vulnerable
111+
// overflow(), this byte is the one clobbered by "*pptr() = ch" when the
112+
// put area is full.
113+
constexpr unsigned char kSentinel = 0xAA;
114+
constexpr unsigned char kOverflowChar = 0x5A; // 'Z'
115+
static_assert(kSentinel != kOverflowChar,
116+
"sentinel and overflow byte must differ to detect the OOB write");
117+
buf->inputBuffer_[0] = static_cast<char>(kSentinel);
118+
119+
// Fill the put area completely. After exactly SIZE sputc() calls,
120+
// pptr() == epptr() but overflow() has not yet been invoked.
121+
for (std::size_t i = 0; i < kSocketBufSize; ++i) {
122+
ASSERT_NE(std::char_traits<char>::eof(), buf->sputc('X'))
123+
<< "sputc failed while filling the put area at index " << i;
124+
}
125+
126+
// The next sputc() must trigger overflow(kOverflowChar). With the
127+
// vulnerable implementation this is where *pptr() = ch writes past the
128+
// end of outputBuffer_ and into inputBuffer_[0].
129+
ASSERT_NE(std::char_traits<char>::eof(),
130+
buf->sputc(static_cast<char>(kOverflowChar)))
131+
<< "sputc returned eof when triggering overflow()";
132+
133+
// After overflow() returns, the put area should have been flushed to the
134+
// socket and the sentinel in inputBuffer_[0] must still be intact.
135+
EXPECT_EQ(kSentinel, static_cast<unsigned char>(buf->inputBuffer_[0]))
136+
<< "basic_socketbuf::overflow() corrupted inputBuffer_[0]: "
137+
<< "wrote ch=0x" << std::hex << static_cast<int>(kOverflowChar)
138+
<< " past the end of outputBuffer_ (SIZE=" << std::dec
139+
<< kSocketBufSize << "). See pcm-sensor-server.cpp "
140+
<< "basic_socketbuf::overflow() (lines 1226-1237): the character "
141+
<< "must be stored only *after* the full put-area has been flushed "
142+
<< "and the put pointers reset.";
143+
144+
// Release the socket before the buf destructor runs sync(); this keeps
145+
// the test output stable regardless of whether the drainer has already
146+
// exited. Resetting buf closes sv[0].
147+
buf.reset();
148+
// sv[1] is closed by the drainer's shutdown.
149+
}

0 commit comments

Comments
 (0)