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

enh enh at google.com
Thu Mar 21 18:37:25 PDT 2024


yeah, that looks neater anyway, and seems to work. thanks!

On Wed, Mar 20, 2024 at 5:56 PM Rob Landley <rob at landley.net> wrote:
>
> 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