Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Limit usage of malloc.conf options to OpenBSD #282

Merged
merged 2 commits into from
Apr 4, 2018
Merged

Limit usage of malloc.conf options to OpenBSD #282

merged 2 commits into from
Apr 4, 2018

Conversation

mptre
Copy link
Owner

@mptre mptre commented Apr 2, 2018

Turns out the S option has a different meaning on {Free,Net}BSD and the R option
is not supported.

/cc @DBOTW and @mike-burns for autoconf inspection

@codecov-io
Copy link

codecov-io commented Apr 2, 2018 •
edited
Loading

Codecov Report

Merging #282 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #282   +/-   ##
=======================================
  Coverage   90.14%   90.14%           
=======================================
  Files           1        1           
  Lines         497      497           
=======================================
  Hits          448      448           
  Misses         49       49

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b7e733...7de61ef. Read the comment docs.

mike-burns
mike-burns reviewed Apr 2, 2018
View reviewed changes
Copy link
Collaborator

@mike-burns mike-burns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

configure.ac Outdated
@@ -15,5 +15,19 @@ AC_SEARCH_LIBS([setupterm], [curses], [], [
)]
)
])
AC_DEFUN([AC_MALLOC_OPTIONS], [
AC_MSG_CHECKING([for malloc hardening options])
case "`uname -s`" in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you AC_CANONICAL_HOST then you can use $host_os in here instead.

@mptre
Copy link
Owner Author

mptre commented Apr 3, 2018

Thanks @mike-burns! Updated diff with your proposal and significant
rework:

  • Make sure to always run AC_SUBST([MALLOC_OPTIONS], ...)

  • Only set MALLOC_OPTIONS when invoking pick so it won't affect any
    other intermediate processes along the process tree leading down to pick

@DBOTW could you confirm that it's working as expected on NetBSD?

@ghost
Copy link

ghost commented Apr 3, 2018

Looks good: checking for netbsdelf7.1 malloc hardening options... no ... 👍

@mptre
Limit usage of malloc.conf options to OpenBSD
6e91e29 
Turns out the S option has a different meaning on {Free,Net}BSD and the R option
is not supported.
@mptre
Ignore two new autoconf files caused by usage of AC_CANONICAL_HOST
8ff60bc
@mptre mptre merged commit 8ff60bc into master Apr 4, 2018
@mptre mptre deleted the malloc-conf branch April 4, 2018 06:19
@mptre
Copy link
Owner Author

mptre commented Apr 4, 2018

@DBOTW thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mike-burns mike-burns mike-burns left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mptre @codecov-io @mike-burns