Re: sensorsd: memory leak and unnecessary exit

James J. Lippard <[email protected]> wrote:

> I have a host which uses sensorsd which periodically stops running, which
> prompted me to dig into the cause, it's one or both of these two issues:
> 
> (1) memory leak on configuration reload, lines 753-758:
> 
> if (cgetstr(buf, "command", &ebuf) < 0)
>     ebuf = NULL;
> if (ebuf != NULL) {
>     p->command = ebuf;
>     ebuf = NULL;
> }
> 
> cgetstr allocates a new ebuf and the previous pointer is overwritten
> without freeing the old one.
> 
> Fix: insert free(p->command) before p->command = ebuf:
> 
> if (cgetstr(buf, "command", &ebuf) < 0)
>     ebuf = NULL;
> if (ebuf != NULL) {
>     free(p->command);
>     p->command = ebuf;
>     ebuf = NULL;
> }
> 
> (2) unnecessary exit in check_sdlim lines 367-368 for individual
> sensor failure that is handled differently from the same code in
> create_sdlim; sensorsd shouldn't be aborting because of a sysctl
> failure on one sensor.
> 
> 		if (sysctl(mib, 5, &sensor, &len, NULL, 0) == -1)
> 			err(1, "sysctl");
> 
> vs. create_sdlim, lines 240-243:
> 
> 			if (sysctl(mib, 5, &sensor, &slen, NULL, 0) == -1) {
> 				if (errno != ENOENT)
> 					warn("sysctl");
> 				continue;
> 			}
> 
> Fix: make check_sdlim code equivalent to create_sdlim:
> 
>                 if (sysctl(mib, 5, &sensor, &len, NULL, 0) == -1) {
> 		        if (errno != ENOENT)
> 			        warn("sysctl");
> 			continue;
> 		}

Like this.

Index: sensorsd.c
===================================================================
RCS file: /cvs/src/usr.sbin/sensorsd/sensorsd.c,v
diff -u -p -u -r1.69 sensorsd.c
--- sensorsd.c	8 Mar 2023 04:43:15 -0000	1.69
+++ sensorsd.c	11 Apr 2026 17:33:22 -0000
@@ -363,8 +363,11 @@ check_sdlim(struct sdlim_t *sdlim, time_
 
 		mib[3] = limit->type;
 		mib[4] = limit->numt;
-		if (sysctl(mib, 5, &sensor, &len, NULL, 0) == -1)
-			err(1, "sysctl");
+		if (sysctl(mib, 5, &sensor, &len, NULL, 0) == -1) {
+			if (errno != ENOENT)
+				warn("sysctl");
+			continue;
+		}
 
 		if (!(limit->flags & SENSORSD_L_ISTATUS)) {
 			enum sensor_status	newastatus = sensor.status;
@@ -753,6 +756,7 @@ parse_config_sdlim(struct sdlim_t *sdlim
 		if (cgetstr(buf, "command", &ebuf) < 0)
 			ebuf = NULL;
 		if (ebuf != NULL) {
+			free(p->command);
 			p->command = ebuf;
 			ebuf = NULL;
 		}