Avoid reading one byte before the path buffer.
ClosedPublic
Actions

Authored by brooks on Jul 21 2020, 11:17 PM.

Details

Reviewers

mckusick
rmacklem
cem
emaste

Commits

rS363673: MFC r363435:
rS363672: MFC r363435:
rS363435: Avoid reading one byte before the path buffer.

Summary

This happens when there's only one component (e.g. "/foo"). This bug
has been present since June 6, 1990 when it was commited to mountd.c
SCCS version 5.9.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32463
Build 29939: arc lint + arc unit

Event Timeline

brooks requested review of this revision.Jul 21 2020, 11:17 PM

brooks created this revision.

Harbormaster completed remote builds in B32463: Diff 74764.Jul 21 2020, 11:17 PM

I tracked this one back to https://github.com/csrg/original-bsd/commit/45cf24f20d90597daa44422eb7497313e70e244a#diff-29db5f32c6d874dca3dcac24c9c68955R565

cem accepted this revision.Jul 22 2020, 12:01 AM

cem added a subscriber: cem.

cem added inline comments.

usr.sbin/mountd/mountd.c
3158	I might swap this one too for visual consistency. No functional difference, I think.

This revision is now accepted and ready to land.Jul 22 2020, 12:01 AM

I concur with the proposed change and also agree that cem's suggestion is a good one.

Revision control history is such an ugly reminder of ones fallibility :-)

Looks fine to me too.
And I see that I can't blame Herb Hasler (the guy who wrote mountd.c when
he was working for me long ago. (Btw, I got email from Herb recently.
He'd doing fine, living in TN these days.)

So, just out of curiosity, did this actually cause a crash or was it detected by some debugging
in malloc() or ???

emaste accepted this revision.Jul 22 2020, 1:11 PM

With CHERI (cheri-cpu.org) this causes a crash as pointers have hardware enforced bounds. This is pretty typical of the sort of long standing bug we're finding. I found a similar one in tcsh a few years ago where hitting <tab> on an empty command line read a byte before the beginning of a string.

I'll make cem's suggested change before commit.

Closed by commit rS363435: Avoid reading one byte before the path buffer. (authored by brooks). · Explain WhyJul 22 2020, 9:45 PM

This revision was automatically updated to reflect the committed changes.

brooks added a commit: rS363435: Avoid reading one byte before the path buffer..

Herald added a subscriber: imp. · View Herald TranscriptJul 22 2020, 9:45 PM

brooks added a commit: rS363672: MFC r363435:.Jul 29 2020, 8:30 PM

brooks added a commit: rS363673: MFC r363435:.