mdmfs: Fix many bugs in automatic md(4) creation.
ClosedPublic
Actions

Authored by brooks on Feb 8 2019, 6:13 PM.

Details

Reviewers

markj
emaste
jhb
kib

Commits

rS344263: MFC r344023:
rS344262: MFC r344023:
rS344023: mdmfs: Fix many bugs in automatic md(4) creation.

Summary

This code allocated a correctly sized buffer, read past the end of the
source buffer, writing off the end of the target buffer, and then writing
a '\0' terminator past the end of the target buffer (in the wrong place).
It then leaked the buffer.

Switch to a statically sized buffer on the stack and update the source pointer and
length before use so the correct things are copied.

Fix a logic error in the checks that the format of the line is as
expected and move on out of an assert.

Remove an unneeded close(). fclose() closes the descriptor.

Found with: CheriABI
Obtained from: CheriBSD

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 22403
Build 21566: arc lint + arc unit

Event Timeline

brooks created this revision.Feb 8 2019, 6:13 PM

Harbormaster completed remote builds in B22394: Diff 53694.Feb 8 2019, 6:13 PM

markj added inline comments.Feb 8 2019, 9:55 PM

sbin/mdmfs/mdmfs.c
484–489	What if the line is just "md\n", i.e., there's no unit number? I think the second clause won't catch that case because linelen includes the newline character. That is, it looks like that should be `linelen <= mdnamelen + 1`.

Make sure there's at least space for one digit of a unit number.

Harbormaster completed remote builds in B22403: Diff 53713.Feb 8 2019, 10:06 PM

brooks marked an inline comment as done.Feb 8 2019, 10:06 PM

It's probably fine, but I wonder if it wouldn't be more readable to just use getline() instead. I think you'd end up with something like:

char *linebuf;
size_t linecap;
...
sfd = fdopen(...);
linebuf = NULL;
linecap = 0;
if (getline(&linebuf, &linecap, sfd) < mdnamelen ||
    strncmp(linebuf, mdname, mdnamelen) != 0)
        errx(1, "unexpected output from mdconfig (attach)");
ul = strtoul(linebuf + mdnamelen, &p, 10);
...
fclose(sfd);
free(linebuf);