[Toybox] [PATCH] mount: avoid deferencing NULL.

Rob Landley rob at landley.net
Wed Mar 20 18:04:50 PDT 2024


On 3/20/24 16:07, enh via Toybox wrote:
> I don't know why I wasn't seeing this yesterday

Because /sys was mounted, so readfile() returned a string with its contents.
(And/or race condition of the mount going away between reading /proc/mounts and
asking for follow-up data about a specific mount point from sysfs.)

Sigh, I initialized ss to "" so I could just printf("%s", ss) without testing,
but readfile() returns NULL when the file doesn't exist and I overwrite it in
place because I didn't want to juggle through a THIRD variable (mostly because
I'm out of convenient names for them), and I missed an else setting it BACK to
"" in the NULL case.

Adding the one test doesn't fix printf() calling null, which segfaults on some
libcs. Lemme put the else in...

The real design failure here is that if the readfile() returns an empty string
we won't free it, but that should never happen, the amount of memory leaked
would be trivial and the command exits at the end of the list.

Hmmm... well, I COULD move the s = xabspath(mm->device, 0) down to the end of
the if (*s == '/') and then use THAT as my third variable...

Ok, I rewrote the code to use three varaibles and thus leave the "" in ss when
it doesn't have reason to change it. (Single Point of truth, setting it BACK to
"" and thus having two "" constants was icky. Yeah, tiny flaw but _I_ saw it.)

Commit d298747580c7 and once again I've only tested the "file exists" path, I'm
not unmounting sysfs on my work laptop and haven't got a convenient test vm I
can loopback mount a filesystem image in at the moment. (The devuan install iso
image I've been using is a bit big to stick in initramfs...)

Rob


More information about the Toybox mailing list