Change image activator to check length of interpreter is less then MAXPATHLEN.
Needs ReviewPublic
Actions

Authored by ambrisko on Jun 6 2018, 10:56 PM.

Details

Reviewers: None

Summary

The image activator checks to see if the space for the path of the interpreter
is larger then MAXPATHLEN. This is bad since it should check the length
of the interpreter is less then MAXPATHLEN. Update the error message to
give a better idea of why a binary won't run.

Also on failure, only free interp_buf if it is not NULL to prevent double
free. Do the VOP_UNLOCK even if it is not locked. I'm not sure about
this but the success part does this so I'm following suite.

Test Plan

Ran cross compile tools that had a long path name to the interpreter that
now works. Didn't do negative testing.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 17055
Build 16918: arc lint + arc unit

Event Timeline

ambrisko created this revision.Jun 6 2018, 10:56 PM

Herald added subscribers: Contributor Reviews (src), imp. · View Herald TranscriptJun 6 2018, 10:56 PM

Harbormaster completed remote builds in B17055: Diff 43392.Jun 6 2018, 10:56 PM

In what situation is this change needed?

sys/kern/imgact_elf.c
851	Now userland can specify an arbitrarily large size to a malloc call.
868	The interp != NULL check doesn't seem to be needed.
869	"too"
1092	How can this be preventing a double free?

Updating D15683: Change image activator to check length of interpreter is less then MAXPATHLEN. #
Enter a brief description of the changes included in this update.
The first line is used as subject, next lines as comment. #
If you intended to create a new revision, use:
$ arc diff --create

Address markj's feedback.

Limit the malloc to the MAXPATHLEN. Add in a failure check after malloc
and not later on. Change "to" to "too".

Clarify that when errors happen, the malloc may not have happened so then
there is no memory to free (ie. goto ret; calls when malloc didn't happen).

Thanks for the feedback.

Harbormaster completed remote builds in B18280: Diff 45749.Jul 23 2018, 10:49 PM

markj added inline comments.Aug 2 2018, 3:22 PM

sys/kern/imgact_elf.c
847	Extra semicolon.
853	malloc(M_WAITOK) can't fail.
1092	This part of the change is not needed.

Get rid of the malloc check and spurious ';'.

Thanks for the feedback.

Harbormaster completed remote builds in B18557: Diff 46248.Aug 3 2018, 4:10 PM

ambrisko marked 5 inline comments as done.Aug 3 2018, 4:17 PM

ambrisko added inline comments.

sys/kern/imgact_elf.c
1092	Are you saying there is no way the goto ret's or interp = imgp->image_header) + phdr[i].p_offset is done bypassing the malloc?

markj added inline comments.Aug 3 2018, 4:21 PM

sys/kern/imgact_elf.c
1092	No, but free(NULL) is a no-op.

Remove NULL check since free does that.

Harbormaster completed remote builds in B18562: Diff 46255.Aug 3 2018, 6:05 PM

ambrisko marked 4 inline comments as done.Aug 3 2018, 6:39 PM

Revision Contents
Changeset List

Path

Size

sys/

kern/

imgact_elf.c

14 lines

Diff 43392

View Options

Change image activator to check length of interpreter is less then MAXPATHLEN.Needs ReviewPublicActions