Hi,
I've found that upsdrvctl crashes when there is not ports= specified ups.conf
#0 __strrchr_sse2 () at ../sysdeps/x86_64/strrchr.S:33
#1 xbasename (file=0x0) at common.c:266
#2 stop_driver (ups=0x1e163e0) at upsdrvctl.c:135
#3 send_all_drivers (command=0x4020f0 <stop_driver>) at upsdrvctl.c:446
#4 main (argc=1, argv=0x7ffff30be9c0) at upsdrvctl.c:565
stop_driver (ups=0x1e163e0) at upsdrvctl.c:135:
if (ret != 0) {
snprintf(pidfn, sizeof(pidfn), "%s/%s-%s.pid", altpidpath(),
ups->driver, xbasename(ups->port));
ret = stat(pidfn, &fs);
}
ups->port is NULL which makes xbasename(NULL) crash. Seems this is the only
problem with wrong configuration. During quick testing I did not find any other
sensitive place or value.
How to reproduce this crash:
1)add ups to ups.conf, omit port configuration
2)run upsdrvctl stop
There is nothing checking ups configuration is ok for upsdrvctl. For daemon
there is check in server/conf.c upsconf_add. I've moved that checking code to
separate function upsconf.c : validate_upsconf and used this function from
upsconf_add and from upsdrvctl.c , this was more intrusive than it seemed. See
attached patch (tested). Simpler way would be just adding validate_upsconf to
upsdrvctl.c but it'd lead to code duplication. Any comments?
Michal
Index: include/upsconf.h
===================================================================
--- include/upsconf.h (revision 2479)
+++ include/upsconf.h (working copy)
@@ -3,5 +3,18 @@
/* open the ups.conf, parse it, and call back do_upsconf_args() */
void read_upsconf(void);
-
+/* remove invalid UPS configurations from upstable */
+void validate_upsconf(void);
+
+typedef struct ups_s {
+ char *upsname;
+ char *driver;
+ char *port;
+ char *desc;
+ int sdorder;
+ int maxstartdelay;
+ struct ups_s *next;
+} ups_t;
+
+extern ups_t * upstable;
\ No newline at end of file
Index: common/upsconf.c
===================================================================
--- common/upsconf.c (revision 2479)
+++ common/upsconf.c (working copy)
@@ -26,7 +26,8 @@
#include "common.h"
#include "parseconf.h"
- static char *ups_section;
+static char *ups_section;
+ups_t *upstable;
/* handle arguments separated by parseconf */
static void conf_args(int numargs, char **arg)
@@ -105,3 +106,48 @@
free(ups_section);
}
+
+/* remove invalid UPS configurations from upstable */
+void validate_upsconf(void)
+{
+ ups_t *tmp = upstable, *next, *old = NULL;
+ upstable = NULL;
+
+ if (!tmp) {
+ upslogx(LOG_WARNING, "Warning: no UPS definitions in ups.conf");
+ return;
+ }
+
+ while (tmp) {
+
+ /* save for later, since we delete as we go along */
+ next = tmp->next;
+
+ /* this should always be set, but better safe than sorry */
+ if (!tmp->upsname) {
+ tmp = tmp->next;
+ continue;
+ }
+
+ /* don't accept an entry that's missing items */
+ if ((!tmp->driver) || (!tmp->port)) {
+ upslogx(LOG_WARNING, "Warning: ignoring incomplete configuration for UPS [%s]\n",
+ tmp->upsname);
+
+ /* free tmp's resources */
+ free(tmp->driver);
+ free(tmp->port);
+ free(tmp->desc);
+ free(tmp->upsname);
+ free(tmp);
+
+ if (old)
+ old->next = next;
+ } else if (!old) {
+ upstable = old = tmp;
+ }
+
+ tmp = next;
+ }
+}
+
Index: server/conf.c
===================================================================
--- server/conf.c (revision 2479)
+++ server/conf.c (working copy)
@@ -319,40 +319,26 @@
/* add valid UPSes from ups.conf to the internal structures */
void upsconf_add(int reloading)
{
- ups_t *tmp = upstable, *next;
+ ups_t *tmp, *next;
char statefn[SMALLBUF];
- if (!tmp) {
- upslogx(LOG_WARNING, "Warning: no UPS definitions in ups.conf");
- return;
- }
+ validate_upsconf();
+ tmp = upstable;
while (tmp) {
/* save for later, since we delete as we go along */
next = tmp->next;
- /* this should always be set, but better safe than sorry */
- if (!tmp->upsname) {
- tmp = tmp->next;
- continue;
- }
+ snprintf(statefn, sizeof(statefn), "%s-%s",
+ tmp->driver, tmp->upsname);
- /* don't accept an entry that's missing items */
- if ((!tmp->driver) || (!tmp->port)) {
- upslogx(LOG_WARNING, "Warning: ignoring incomplete configuration for UPS [%s]\n",
- tmp->upsname);
- } else {
- snprintf(statefn, sizeof(statefn), "%s-%s",
- tmp->driver, tmp->upsname);
+ /* if a UPS exists, update it, else add it as new */
+ if ((reloading) && (get_ups_ptr(tmp->upsname) != NULL))
+ ups_update(statefn, tmp->upsname, tmp->desc);
+ else
+ ups_create(statefn, tmp->upsname, tmp->desc);
- /* if a UPS exists, update it, else add it as new */
- if ((reloading) && (get_ups_ptr(tmp->upsname) != NULL))
- ups_update(statefn, tmp->upsname, tmp->desc);
- else
- ups_create(statefn, tmp->upsname, tmp->desc);
- }
-
/* free tmp's resources */
free(tmp->driver);
Index: server/conf.h
===================================================================
--- server/conf.h (revision 2479)
+++ server/conf.h (working copy)
@@ -28,14 +28,6 @@
/* flush existing config, then reread everything */
void conf_reload(void);
-typedef struct ups_s {
- char *upsname;
- char *driver;
- char *port;
- char *desc;
- struct ups_s *next;
-} ups_t;
-
/* used for really clean shutdowns */
void delete_acls(void);
void delete_access(void);
Index: drivers/upsdrvctl.c
===================================================================
--- drivers/upsdrvctl.c (revision 2479)
+++ drivers/upsdrvctl.c (working copy)
@@ -29,17 +29,6 @@
#include "common.h"
#include "upsconf.h"
-typedef struct {
- char *upsname;
- char *driver;
- char *port;
- int sdorder;
- int maxstartdelay;
- void *next;
-} ups_t;
-
-static ups_t *upstable = NULL;
-
static int maxsdorder = 0, testmode = 0, exec_error = 0;
/* timer - keeps us from getting stuck if a driver hangs */
@@ -101,6 +90,7 @@
tmp = xmalloc(sizeof(ups_t));
tmp->upsname = xstrdup(upsname);
tmp->driver = NULL;
+ tmp->desc = NULL;
tmp->port = NULL;
tmp->next = NULL;
tmp->sdorder = 0;
@@ -404,6 +394,7 @@
free(tmp->driver);
free(tmp->port);
free(tmp->upsname);
+ free(tmp->desc);
free(tmp);
tmp = next;
@@ -489,6 +480,7 @@
atexit(exit_cleanup);
read_upsconf();
+ validate_upsconf();
if (argc == 1)
send_all_drivers(command);
_______________________________________________
Nut-upsdev mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev
