Nut upsdev - Jul 2010 - [PATCH] nut crashes when port= is omitted

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nut-portcrash.patch
Type: text/x-patch
Size: 5024 bytes
Desc: not available
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20100721/e4e42d35/attachment.bin>

Citeren Michal Hlavinka <mhlavink op redhat.com>:
> 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?
Thanks for your patch.

This part of the code if used for stopping legacy drivers (up to and  
including nut-2.0.4), which are by now more than four years old. Newer  
drivers (nut-2.0.5 and later) use a different naming scheme for the  
driver sockets (and PID file) and the value of 'port' is no longer  
used anymore for those.

Instead of validating the value of 'port', it is sufficient to check  
for NULL if the PID file is not in the nut-2.0.5 location. This also  
allows us to skip the 'port' variable where this is not needed (for  
instance the usbhid-ups driver). A revised version is now available in  
the SVN trunk.

Best regards, Arjen
-- 
Please keep list traffic on the list