Skip to content

Commit bd9997b

Browse files
authored
sensors: make getsensoridentity/getsensorshort thread-safe (fix garbage names under concurrent callers) (#183)
* sensors: serialise getsensor* and stop getsensorid falling through getsensoridentity()/getsensorshort() drive i2c detection through the global i2c_adapter_nr and format into one shared static buffer, unlocked; getsensorid() also fell off the end with no return when every bus failed. Concurrent callers stomped the adapter mid-probe and tore the buffer, and the missing return let an all-fail probe format an uninitialised ctx — together producing binary-garbage sensor names. Serialise detect+format under one mutex and return false on the exhausted path. * build: link Threads for the sensor-detection mutex sensors.c now uses pthread_mutex_*, the first real pthread use in the tree, but the targets only linked m. It happens to resolve on musl and glibc >= 2.34 (pthread folded into libc) yet fails to link on older glibc and uClibc. find_package(Threads REQUIRED) and link Threads::Threads on ipchw (interface, so ipcinfo and external consumers of libipchw pull it in) and on ipctool.
1 parent f6e64a1 commit bd9997b

2 files changed

Lines changed: 39 additions & 13 deletions

File tree

CMakeLists.txt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ project(ipctool C)
44
set(CMAKE_C_STANDARD 99)
55
set(CMAKE_C_FLAGS "-std=gnu99")
66

7+
# sensors.c serialises sensor detection with a pthread mutex. On musl and
8+
# glibc >= 2.34 the pthread symbols live in libc, but older glibc and uClibc
9+
# need the explicit link flag, so ask for it rather than rely on the toolchain.
10+
find_package(Threads REQUIRED)
11+
712
cmake_policy(SET CMP0069 NEW)
813

914
if(NOT BUILD_SHARED_LIBS)
@@ -181,13 +186,15 @@ set(CYAML_TEST_SRC
181186

182187
add_library(ipchw STATIC ${COMMON_LIB_SRC})
183188
target_compile_definitions(ipchw PUBLIC STANDALONE_LIBRARY ${IPCHW_VENDOR_DEFS})
184-
target_link_libraries(ipchw m)
189+
# Threads is carried on the interface so anything linking this static library
190+
# (ipcinfo here, and external consumers of libipchw) pulls pthread in too.
191+
target_link_libraries(ipchw m Threads::Threads)
185192

186193
if(NOT ONLY_LIBRARY)
187194
add_executable(ipctool ${IPCTOOL_SRC} ${COMMON_LIB_SRC_ALL})
188195
target_compile_definitions(ipctool PRIVATE ${IPCHW_ALL_VENDOR_DEFS})
189196

190-
target_link_libraries(ipctool m)
197+
target_link_libraries(ipctool m Threads::Threads)
191198
install(TARGETS ipctool RUNTIME DESTINATION /usr/bin/)
192199

193200
add_executable(ipcinfo example/ipcinfo.c src/tools.c ${VERSION_SRC})

src/sensors.c

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,10 +1245,13 @@ if (!open_i2c_sensor_fd(i2c_adapter_nr))
12451245
}
12461246

12471247

1248-
1249-
}
12501248

1249+
}
12511250

1251+
/* All buses probed, nothing answered. Without this the function fell off
1252+
* the end (UB): the bool came back indeterminate — often true — and the
1253+
* caller then formatted an uninitialised ctx into a garbage name. */
1254+
return false;
12521255
}
12531256

12541257
#ifndef STANDALONE_LIBRARY
@@ -1287,20 +1290,36 @@ cJSON *detect_sensors() {
12871290

12881291
#endif
12891292

1293+
/* getsensorid() drives i2c detection through the process-global i2c_adapter_nr
1294+
* and both entry points below format into one shared sensor_indentity buffer,
1295+
* none of it locked. Two threads calling these at once stomp the adapter number
1296+
* mid-probe (wrong bus -> detection fails -> garbage) and tear the buffer write.
1297+
* Serialise the whole detect-and-format so every returned value is one complete,
1298+
* valid string. */
1299+
static pthread_mutex_t sensor_indentity_mtx = PTHREAD_MUTEX_INITIALIZER;
12901300
static char sensor_indentity[16];
12911301
const char *getsensoridentity() {
1302+
pthread_mutex_lock(&sensor_indentity_mtx);
12921303
sensor_ctx_t ctx;
1293-
if (!getsensorid(&ctx))
1294-
return NULL;
1295-
lsnprintf(sensor_indentity, sizeof(sensor_indentity), "%s_%s",
1296-
ctx.sensor_id, ctx.control);
1297-
return sensor_indentity;
1304+
const char *ret = NULL;
1305+
if (getsensorid(&ctx)) {
1306+
lsnprintf(sensor_indentity, sizeof(sensor_indentity), "%s_%s",
1307+
ctx.sensor_id, ctx.control);
1308+
ret = sensor_indentity;
1309+
}
1310+
pthread_mutex_unlock(&sensor_indentity_mtx);
1311+
return ret;
12981312
}
12991313

13001314
const char *getsensorshort() {
1315+
pthread_mutex_lock(&sensor_indentity_mtx);
13011316
sensor_ctx_t ctx;
1302-
if (!getsensorid(&ctx))
1303-
return NULL;
1304-
lsnprintf(sensor_indentity, sizeof(sensor_indentity), "%s", ctx.sensor_id);
1305-
return sensor_indentity;
1317+
const char *ret = NULL;
1318+
if (getsensorid(&ctx)) {
1319+
lsnprintf(sensor_indentity, sizeof(sensor_indentity), "%s",
1320+
ctx.sensor_id);
1321+
ret = sensor_indentity;
1322+
}
1323+
pthread_mutex_unlock(&sensor_indentity_mtx);
1324+
return ret;
13061325
}

0 commit comments

Comments
 (0)