nut: hotplug script includes leading zeroes in PRODUCT case, leading to undetected hw

[tofurky]

Maintainer: @cshoredaniel
Environment: ath79, ubnt,routerstation-pro, r8011+2-6bbb220255

'nut' package was compiled from latest commit as of 9/7/18 (2e624f9)

Description:

/etc/hotplug.d/usb/30-libhid-ups has a case statement like so:

[ -n "$PRODUCT" ] && case "$PRODUCT" in
3f0/0001/* | \
3f0/1f01/* | \
3f0/1f02/* | \
3f0/1f06/* | \
...
50d/0900/* | \
50d/0910/* | \
...
"")
	[ -d /var/run/nut ] && [ ! -f /var/run/nut/disable-hotplug ] && \
		/etc/init.d/nut-server enabled &&  perform_libhid_action
	;;
esac

however, adding "env >> /tmp/debug" to the script showed that PRODUCT does not include the leading 0 for my ups:

pvendid=50d pprodid=910
USER=root
ACTION=bind
...
PRODUCT=50d/910/6
INTERFACE=3/0/0
PWD=/
DEVTYPE=usb_interface

so the case doesn't match and the hotplug script doesn't set permissions, etc. leading to brokenness when not running upsd and /lib/nut/usbhid-ups as root.

the fix seems obvious, remove the leading zeroes. however i'm unsure if all leading zeroes should be removed, or just the first. in my case of PRODUCT=50d/910/6, changing the relevant case line to 50d/910/* | \ fixed the issue and allowed the hotplug script to run as intended.

also, i'll throw this out there as well - /etc/init.d/nut-server seems to unconditionally set local RUNAS=nut in start_driver_instance(), but for some reason still has this later on:

	if [ -n "$RUNAS" ]; then
		chown $RUNAS:$(id -gn $RUNAS) "${STATEPATH}"
		chown $RUNAS:$(id -gn $RUNAS) "$(dirname "$UPS_C")"
	fi

[tofurky]

seems the issue is in the sed expression in the Makefile.

changing:

        $(SED) '/^$$$$/d' \
                -e '/^#/d' \
                -E -e 's:^[^ ][^ ]*  *0x0003  *0x0{0,3}([^ ][^ ]*)  *0x{0,3}*([^ ][^ ]*).*:\1/\2/* | \\:' \
                $(PKG_BUILD_DIR)/30-libhid-ups.middle

to:

        $(SED) '/^$$$$/d' \
                -e '/^#/d' \
                -E -e 's:^[^ ][^ ]*  *0x0003  *0x0{0,3}([^ ][^ ]*)  *0x0{0,3}*([^ ][^ ]*).*:\1/\2/* | \\:' \
                $(PKG_BUILD_DIR)/30-libhid-ups.middle

seems to make the hotplug script usable for my ups.

specifically:
*0x0{0,3}([^ ][^ ]*) *0x{0,3}*([^ ][^ ]*)
to:
*0x0{0,3}([^ ][^ ]*) *0x0{0,3}*([^ ][^ ]*)

[danielfdickinson]

@tofurky Ooops....yeah the VendorId is right (strips leading 0), but ProductId is wrong (and of course my hardware doesn't have a leading zero on the ProductId so I didn't catch it).

Also, for the RUNAS, if you look carefully, there is a function called between the local and use of runas that sets runas to what's in the config file.

[tofurky]

ah yeah i see build_config now. thanks for the quick response!

[tofurky]

i think i noticed another issue with the hotplug script. it has the following which is called early in boot, before the init script runs (and before /var/run/nut is created):

"")
	[ -d /var/run/nut/ ] && [ ! -f /var/run/nut/disable-hotplug ] && \
		/etc/init.d/nut-server enabled &&  perform_libhid_action
	;;
esac

either removing [ -d /var/run/nut/ ] or adding mkdir -p /var/run/nut seems to fix it and allows the driver to start.

[danielfdickinson]

Hrrm....I'll have to modify that to make sure hotplugging doesn't happen during FSD, but good catch; thanks!

[dibdot]

@cshoredaniel can we close this right now?

[tofurky]

@dibdot the fixes haven't been merged yet from what i can see

[danielfdickinson]

@dibdot Still in progress....will include the issue # in commit and it will auto-close once merged.

[danielfdickinson]

@tofurky FYI I've posted PR: #7638 which ought to do must of what's now in master except the USB hotplug bits (as that's a new feature IMO). If you can, please test and comment!