Page MenuHomeFreeBSD

mount(8): Avoid truncation when fstab-formatting unionfs mount info
ClosedPublic

Authored by jah on Sun, Dec 22, 7:01 AM.
Tags
None
Referenced Files
F107375382: D48177.diff
Mon, Jan 13, 6:21 AM
Unknown Object (File)
Fri, Jan 10, 4:20 PM
Unknown Object (File)
Mon, Dec 30, 12:41 AM
Unknown Object (File)
Sat, Dec 28, 8:22 PM
Unknown Object (File)
Fri, Dec 27, 11:29 AM
Unknown Object (File)
Fri, Dec 27, 9:01 AM
Unknown Object (File)
Fri, Dec 27, 8:02 AM
Unknown Object (File)
Mon, Dec 23, 4:28 PM
Subscribers

Details

Summary

When displaying unionfs mounts in fstab format (mount -p), mount(8)
currently uses strlcpy to remove the disposition prefix from the mount
name returned by getmntinfo(3). But strlcpy, like strcpy before it,
does not guaranteee correct behavior if the source and destination
buffers overlap.

Use memmove for this case instead.

PR: 283420
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 61303
Build 58187: arc lint + arc unit

Event Timeline

jah requested review of this revision.Sun, Dec 22, 7:01 AM
sbin/mount/mount.c
913

Is there any concern about handling buffers that aren't NUL-terminated here?
I would guess not, since it seems that would be a serious failure on the part of the kernel to correctly populate the string (and there certainly doesn't seem to be such concern in the rest of this file), but I have to ask...

imp added inline comments.
sbin/mount/mount.c
913

Just 3 lines later we take atrlen of it again. And other options. These are actual arrays, not pointers to strings, so i think the worst case of malformed data would just produce garbled output for other fields.

But we trust the kernel as a source of data on a lot of places. The original strlcpy is clearly wrong and i would have used a pointer to this field below and offser it in the above/below: cases to avoid the copy.

This revision is now accepted and ready to land.Sun, Dec 22, 10:28 AM
sbin/mount/mount.c
913

Yep, I noticed the same things.

I'd been assuming there was some other reason we didn't want to just offset the pointer, but it really doesn't look as though there is. It also seems dumb to destructively modify the buffer like this in case we do later end up wanting the original contents for some reason.

This revision now requires review to proceed.Sun, Dec 22, 2:14 PM
This revision is now accepted and ready to land.Sun, Dec 22, 5:57 PM