#1033716 watchdog: Watchdog incorrectly thinks iface eth0 not receiving any data

Package:
watchdog
Source:
watchdog
Description:
system health checker and software/hardware watchdog handler
Submitter:
Pirate
Date:
2023-04-02 22:03:07 UTC
Severity:
normal
#1033716#5
Date:
2023-03-30 21:34:24 UTC
From:
To:

#1033716#10
Date:
2023-03-31 02:46:35 UTC
From:
To:
Hello,
After doing certain activities, in my case, restoring an external SD card
from a backup using a program, watchdog thinks that
"device eth0 did not receive anything since last check"
which triggers a reboot as I've configured in Watchdog.
However, eth0 is working fine, /proc/net/dev is incrementing, the interface
is working, I'm serving HTTPS/HTTPS/DNS over this interface, I'm SShed in
on the interface.
/dev/proc/net is incrementing, I am cat'ing it and watch it increment AS
watchdog says its not receiving anything

Relevant links:
https://github.com/RPi-Distro/repo/issues/237#issuecomment-1490751380

#1033716#15
Date:
2023-04-02 20:13:58 UTC
From:
To:
I have found more information on this. Not sure if it is a problem with the
watchdog package or how the system is configured, but the problem is that
there is an overflow of the "bytes" variable in the "iface.c" source file
for the watchdog.

This is the line that is causing the issues:
unsigned long bytes = strtoul(line + i + strlen(dev->name) + 1, NULL, 10);

Reference:
https://github.com/RPi-Distro/repo/issues/237#issuecomment-1493429567

#1033716#20
Date:
2023-04-02 21:59:52 UTC
From:
To:
I think the problem is that the '/proc/net/dev' command returns 'unsigned
long long' (see here:
https://github.com/raspberrypi/linux/blame/rpi-6.1.y/net/core/net-procfs.c#L82),
but the watchdog 'iface.c' code uses 'unsigned long', which causes the
'bytes' to always be 4294967295 (0xFFFF FFFF or ULONG_MAX). There is no
check that the 'strtoul' operation has been successful.

Recompiling the watchdog using 'unsigned long long' for 'bytes' and
changing 'strtoul' to 'stroull' apparently solved the problem for me after
doing a quick test. See changes below.


I would add the additional checks for "strtoull" to at least add a warning
in the logs if this is happening in the future. Maybe even better it would
be to use 'u64' instead of 'unsigned long long', as that's what is stored
anyway in "struct rtnl_hw_stats64" where "rx_bytes" that we read are
stored. However, they are printed as "%llu" so maybe better to keep as is?

diff --git a/iface.c.orig b/iface.c
index 5db4e55..7b5eba6 100644
--- a/iface.c.orig
+++ b/iface.c
@@ -41,11 +41,11 @@ int check_iface(struct list *dev)

                        for (; line[i] == ' ' || line[i] == '\t'; i++) ;
                        if (strncmp(line + i, dev->name, strlen(dev->name))
== 0) {
-                               unsigned long bytes = strtoul(line + i +
strlen(dev->name) + 1, NULL, 10);
+                               unsigned long long bytes = strtoull(line +
i + strlen(dev->name) + 1, NULL, 10);

                                /* do verbose logging */
                                if (verbose && logtick && ticker == 1)
-                                       log_message(LOG_DEBUG, "device %s
received %lu bytes", dev->name, bytes);
+                                       log_message(LOG_DEBUG, "device %s
received %llu bytes", dev->name, bytes);

                                if (dev->parameter.iface.bytes == bytes) {
                                        fclose(file);

diff --git a/extern.h b/extern.h.orig
index 2eccf0b..81bc620 100644
--- a/extern.h
+++ b/extern.h.orig
@@ -30,7 +30,7 @@ struct filemode {
 };

 struct ifmode {
-       unsigned long long bytes;
+       unsigned long bytes;
 };

 struct tempmode {