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

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #74764)	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:.