[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#873922: [PATCH] Test for NULL file pointer for /sys/class/net/<ifname>/carrier



I can confirm that netcfg crashes in this situation, tested with version
1.189 (UNRELEASED) (git a218ebabc8cafdf5127e7ae006d3be15feef8824)

I used the following method for testing and debugging this.

You will need:
- a clean build environment like a container or VM (I used Incus)
- a basic VM to test on (qemu/libvirt)

Set up Incus (following https://wiki.debian.org/Incus)

incus create images:debian/12 debcon

(Some additional steps to include home dir bind mount etc. skipped here.)

Inside container:

Update /etc/apt/sources.list to include unstable deb and deb-src.

Set up the debian installer sources following https://wiki.debian.org/DebianInstaller/Build

In debian-installer/packages/netcfg:

(edit source files to include more debugging)

dpkg-buildpackage -b --no-sign

In debian-installer/installer/build:

copy the udeb file to localudebs/

cp ../../packages/netcfg_1.189+test1_amd64.udeb localudebs/netcfg.udeb

fakeroot make build_netboot

Now copy the kernel and initrd to a location outside the container
(easiest if a bind mount was used, otherwise incus file pull can be used).

cd /var/tmp
cp debian-installer/installer/build/dest/netboot/debian-installer/amd64/initrd.gz .
cp debian-installer/installer/build/dest/netboot/debian-installer/amd64/linux .

Use virt-manager to create a new VM:

- type QEMU/KVM
- Manual Installation
- debiantesting
- default mem/cpu
- check the box for 'edit configuration before launch'

In the hardware settings, add a second network interface
In the boot settings, select the kernel and initrd in /var/tmp
set the kernel boot parameters to "interface=invalid BOOT_DEBUG=2 auto=true"

Start the installation procedure. For me this crashed netcfg.

After adding more debugging to the code I found that ethtool-lite.c was
opening /sys/class/net/invalid/carrier but that file does not exist so
the file pointer returned is NULL. The code then proceed to fgets from
the NULL pointer.

Below is a small patch that remedies that situation.

AS the installer would get stuck retrying the same interface name, the
patch adds code to reset netcfg/choose_interface in case UNKNOWN is
returned by ethtool_lite.

---
 ethtool-lite.c  |  4 ++++
 netcfg-common.c | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/ethtool-lite.c b/ethtool-lite.c
index a5f74c56..d30f8451 100644
--- a/ethtool-lite.c
+++ b/ethtool-lite.c
@@ -46,6 +46,10 @@ int ethtool_lite (const char * iface)
 	char* filename = malloc(len);
 	snprintf(filename, len, SYSCLASSNET "%s/carrier", iface);
 	FILE* fp = fopen(filename, "r");
+        if (!fp) {
+            di_warning("Could not open %s: %s", filename, strerror(errno));
+            return UNKNOWN;
+        }
 	free(filename);
char result[2];
diff --git a/netcfg-common.c b/netcfg-common.c
index f6006be3..a8889b5d 100644
--- a/netcfg-common.c
+++ b/netcfg-common.c
@@ -1486,6 +1486,7 @@ int netcfg_detect_link(struct debconfclient *client, const struct netcfg_interfa
     int gw_tries = NETCFG_GATEWAY_REACHABILITY_TRIES;
     const char *if_name = interface->name;
     const char *gateway = interface->gateway;
+    int connected; /* as returned by ethtool_lite */
if (!empty_str(gateway))
         sprintf(arping, "arping -c 1 -w 1 -f -I %s %s", if_name, gateway);
@@ -1538,7 +1539,8 @@ int netcfg_detect_link(struct debconfclient *client, const struct netcfg_interfa
             di_info("Detecting link on %s was cancelled", if_name);
             break;
         }
-        if (ethtool_lite(if_name) == 1) /* ethtool-lite's CONNECTED */ {
+        connected = ethtool_lite(if_name);
+        if (connected == 1) /* ethtool-lite's CONNECTED */ {
             di_info("Found link on %s", if_name);
if (!empty_str(gateway) && !is_wireless_iface(if_name) && !is_layer3_qeth(if_name)) {
@@ -1551,6 +1553,13 @@ int netcfg_detect_link(struct debconfclient *client, const struct netcfg_interfa
rv = 1;
             break;
+        } else if (connected == 3) /* ethtool-lite UNKNOWN */ {
+            /* There can be multiple reasons for this situation but
+               the safest course is perhaps to forget
+               netcfg/choose_interface and drop out */
+            di_info("Unknown interface name %s", if_name);
+            debconf_reset(client, "netcfg/choose_interface");
+            break;
         }
     }
--
2.39.2


Reply to: