aranemac / munin-smart-nvme

Converts smartctl output for NVMe drives to old format (readable for munin smart_ plugin)
MIT License
5 stars 1 forks source link

Losing passed arguments breaks support for many NVMe drives #2

Open mgc8 opened 11 months ago

mgc8 commented 11 months ago

Thank you for this very useful converter!

In using it on a number of different NVMe drives, I noticed that it breaks on any that don't work "automatically" with smartctl (e.g. the ones that need a special "-d" parameter). This is caused by the "overwrite all arguments" code, which is broken and unnecessary.

Here is a short fix that allows all arguments to be passed correctly, as configured by munin (and it also fixes the potential buffer overflow with the naked strcat):

--- smartctlcvt.cc  2024-01-07 00:22:06.099416365 +0100
+++ smartctlcvt-fixed.cc    2024-01-07 00:48:12.905155998 +0100
@@ -23,9 +23,13 @@
     char *pT;

     FILE *cmd;
-    // overwriting all arguments passed by plugin, except drive
-    char smartarg[100] = "smartctl -A ";
-    strcat( smartarg, argv[ argc-1 ] );
+    // reconstruct all arguments passed by plugin
+    const int MAXARG = 100;
+    char smartarg[MAXARG] = "smartctl";
+    for ( int i = 1; i < argc; i++ ) {
+   strncat( smartarg, " ", MAXARG - strlen(smartarg) - 1 );
+   strncat( smartarg, argv[i], MAXARG - strlen(smartarg) - strlen(argv[i]) );
+    }

     int on = 0;
     int n = 0;

This works nicely on my system with 10+ devices, some connected directly, others via USB.

aranemac commented 11 months ago

Thanks for the hint! I changed it. :-)

mgc8 commented 11 months ago

Thank you for the quick reply, it looks great! Very useful tool, wish it was part of the main plugin :)