-
-
Notifications
You must be signed in to change notification settings - Fork 430
sudo upscmd -u XXX -p XXX test.battery.start.XXX all type causing segmentation false #3436
Copy link
Copy link
Open
Labels
AIFor good or bad, machine tools are upon us. Humans are still the responsible ones.For good or bad, machine tools are upon us. Humans are still the responsible ones.Qx protocol driverDriver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some othersDriver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some othersUPSilon2000Devices bundled with UPSilon2000 monitoring softwareDevices bundled with UPSilon2000 monitoring softwareUSB VID=0001 PID=0000 (Fry's Electronics/MEC0003)Seems to be a generic USB chip interfacing many devices and protocols (Qx, USB HID, ATCL...)Seems to be a generic USB chip interfacing many devices and protocols (Qx, USB HID, ATCL...)bugimpacts-release-2.8.1Issues reported against NUT release 2.8.1 (maybe vanilla or with minor packaging tweaks)Issues reported against NUT release 2.8.1 (maybe vanilla or with minor packaging tweaks)
Milestone
Metadata
Metadata
Assignees
Labels
AIFor good or bad, machine tools are upon us. Humans are still the responsible ones.For good or bad, machine tools are upon us. Humans are still the responsible ones.Qx protocol driverDriver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some othersDriver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some othersUPSilon2000Devices bundled with UPSilon2000 monitoring softwareDevices bundled with UPSilon2000 monitoring softwareUSB VID=0001 PID=0000 (Fry's Electronics/MEC0003)Seems to be a generic USB chip interfacing many devices and protocols (Qx, USB HID, ATCL...)Seems to be a generic USB chip interfacing many devices and protocols (Qx, USB HID, ATCL...)bugimpacts-release-2.8.1Issues reported against NUT release 2.8.1 (maybe vanilla or with minor packaging tweaks)Issues reported against NUT release 2.8.1 (maybe vanilla or with minor packaging tweaks)
nutdrv_qx: SIGSEGV in main_instcmd() when INSTCMD sent without extra parameter
Environment
Description
When
upscmdsends anINSTCMDwith no extra parameter (e.g.test.battery.start.quick),the driver crashes with SIGSEGV inside
__vsnprintf_chk.Steps to Reproduce
Expected Behavior
Battery test executes. Driver continues running.
Actual Behavior
Driver crashes with segmentation fault.
Root Cause
In
drivers/dstate.c,sock_arg()correctly setscmdparamtoNULLwhen no extraparameter is present (i.e.
numarg == 2), but then passesarg[2]— a garbage/uninitializedpointer — directly to
main_instcmd()instead of usingcmdparam:main_instcmd()indrivers/main.cthen passes this garbage pointer toupsdebugx():vsnprintfattempts to dereference0x26as a string pointer →SIGSEGV.GDB Stack Trace
Thread 1 "nutdrv_qx" received signal SIGSEGV, Segmentation fault.
#3 __vsnprintf_chk () from /lib/aarch64-linux-gnu/libc.so.6
#4 vupslog (fmt="[D2] entering main_instcmd(%s, %s) for [%s] on %s") at common.c:1283
#5 main_instcmd (cmdname="test.battery.start.quick",
extra=0x26 <error: Cannot access memory at address 0x26>,
conn=0x555569fcc0) at main.c:700
#6 sock_arg (conn=0x555569fcc0) at dstate.c:783
#7 sock_read (conn=0x555569fcc0) at dstate.c:920
#8 dstate_poll_fds () at dstate.c:1094
#9 main () at main.c:2448
Fix
drivers/dstate.cline 783 — usecmdparam(which is already correctly set toNULLwhen no parameter is present) instead of raw
arg[2]:Optionally, also harden the debug log in
drivers/main.cline 700:Additional Notes
This UPS also has a related issue: the
Icommand returns all-spaces for the firmwarefield, causing
nutdrv_qxMegatec protocol probe to fail withDevice not supported!even though Q1 responses are valid. Workaround: add
novendortoups.conf.A proper fix would be adding
QX_FLAG_ABSENTto theups.firmwareentry indrivers/nutdrv_qx_megatec.cline 78.This bug was fixed by Sonnet 4.6 ,Atleast it not crashed