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.

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

OpenBSD -current has removed mincore syscall -- lwt no longer builds #663

Closed
krwesterback opened this issue Feb 17, 2019 · 8 comments
Closed

Comments

@krwesterback
Copy link
Contributor

To quote from the OpenBSD commit message:

"mincore() is a relic from the past, exposing physical machine information about shared resources which no program should see."

As a result lwt no longer builds on OpenBSD -current, and thus the upcoming 6.5.

I don't see any use of mincore(), so perhaps it is just included in unix stubs in pursuit of
completeness. I doubt if OpenBSD will be the last OS to remove mincore() as it has been identified as a security issue and vector for exploitation.

Perhaps simply expanding the check

#ifdef CYGWIN
LWT_NOT_AVAILABLE4(unix_mincore)

would be sufficient for now. I have not yet had a chance to try that, or find out the appropriate define to use.

@aantron
Copy link
Collaborator

aantron commented Feb 17, 2019

@krwesterback Expanding this check should indeed be sufficient. If you could try it and submit a PR, it would be appreciated, as most of us don't have immediate access to an OpenBSD machine, though we could, of course, set it up if necessary :)

If you can check for the OpenBSD version, I think that would be best. That might make the whole check slightly complicated. You may have to resort to something like

#ifdef CYGWIN
#define NO_MINCORE
#endif

#ifdef // Complicated OpenBSD version check
#define NO_MINCORE
#endif

#ifdef NO_MINCORE
LWT_NOT_AVAILABLE(unix_mincore)
#else
// The actual definition
#endif

In any case, thanks for the report :)

@krwesterback
Copy link
Contributor Author

Pin'ing lwt 4.1.0 and expanding the #ifdef in unix_mincore.c with

#ifdef CYGWIN
LWT_NOT_AVAILABLE4(unix_mincore)
+#elif defined OpenBSD
+LWT_NOT_AVAILABLE4(unix_mincore)
#else

allows lwt to compile. But, no matter how much I hack at every copy of unix_mincore.c (and there seem to be many in ~/.opam) things that use lwt (e.g. lambda-term) won't build because they insist unix_mincore.c refers to mincore at line 33.

Well beyond my ocaml/opam/etc expertise to diagnose further. :-( Happy to test any
suggested fixes.

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] The compilation of lambda-term failed at "/home/krw/.opam/4.07.1/bin/jbuilder build -p lambda-term
-j 1".

#=== ERROR while compiling lambda-term.1.13 ===================================#

context 2.0.3 | openbsd/x86_64 | ocaml-base-compiler.4.07.1 | https://opam.ocaml.org#68cdeb96

path ~/.opam/4.07.1/.opam-switch/build/lambda-term.1.13

command ~/.opam/4.07.1/bin/jbuilder build -p lambda-term -j 1

exit-code 1

env-file ~/.opam/log/lambda-term-41838-58c514.env

output-file ~/.opam/log/lambda-term-41838-58c514.out

output

[...]

Note: You can use "dune upgrade" to convert your project to dune.

File "tools/jbuild", line 1, characters 0-0:

Warning: jbuild files are deprecated, please convert this file to a dune file instead.

Note: You can use "dune upgrade" to convert your project to dune.

ocamlopt tools/lambda_term_actions.exe (exit 2)

(cd _build/default && /home/krw/.opam/4.07.1/bin/ocamlopt.opt -w -40 -safe-string -g -o tools/lambda_term_actions.exe -I /home/krw/.opam/4.07.1/lib/bisect_ppx/runtime -I /home/krw/.opam/4.07.1/lib/bytes -I /home/krw/.opam/4.07.1/lib/camomile -I /home/krw/.opam/4.07.1/lib/camomile/default_config -I /home/krw/.opam/4.07.1/lib/camomile/dyn -I /home/krw/.opam/4.07.1/lib/camomile/lib_default -I /h[...]

ld: error: undefined symbol: mincore

>>> referenced by unix_mincore.c:33

>>> unix_mincore.o:(lwt_unix_mincore) in archive /home/krw/.opam/4.07.1/lib/lwt/unix/liblwt_unix_stubs.a

cc: error: linker command failed with exit code 1 (use -v to see invocation)

File "caml_startup", line 1:

Error: Error during linking

@aantron
Copy link
Collaborator

aantron commented Feb 21, 2019

This looks like a problem with how you're editing Lwt. If your patch had been picked up in your opam switch, the line number would have been 35 (I believe), not 33. What commands are you using to install the patched Lwt?

It should be something like

git clone https://github.com/ocsigen/lwt.git
cd lwt
# Apply the patch.
opam pin add lwt . --kind=path

This gets you the source of Lwt master, not 4.1.0. The PR should be against master anyway, and no breaking changes have been applied since 4.1.0.

The pin command will pin and install Lwt from that source (with your patch).

Things should work after that, at least you shouldn't get this error on line 33.

@aantron
Copy link
Collaborator

aantron commented Feb 21, 2019

You shouldn't have to directly edit anything in .opam, by the way.

@krwesterback
Copy link
Contributor Author

Bingo! Following your steps lets lwt and lambda-term build without complaint.

What I did was follow the steps I spotted in the opam manual,

opam source --dev-repo --pin
cd ; hack hack hack;
opam upgrade .

Although looking at it now, I suspect I was not including that "." in the opam upgrade statement.

I was aware that changing files inside .opam is not the way to go. I was just thrashing about trying to find something that changed the output. :-)

Examining the entrails I also now suspect that I should have paid more attention to the
message

"[NOTE] Ignoring uncommitted changes in /home/krw/lwt (`--working-dir' not active)."

Anyway, I'll now move on to try figuring out how to create a PR, which I assume means "Pull Request" in this context. :-)

Thanks for the cluebat!

@aantron
Copy link
Collaborator

aantron commented Feb 21, 2019

Great :)

The only thing I'd ask, is that in the PR, instead of only

#ifdef OpenBSD

for the check, we have the condition check for OpenBSD and that the version is at least whatever version will remove mincore. We don't want to break Lwt's mincore on existing versions of OpenBSD, where it was working, and someone may actually be using it (however unlikely).

Could you also provide a link to the OpenBSD message about the removal of mincore?

@krwesterback
Copy link
Contributor Author

I will look into the version question as I investigate PR creation. It is likely to be complicated by the fact that the change takes place in -current and not yet in a release.

I will include a link.

I would also argue that mincore() has been determined to be a security problem with no counterbalancing advantage, and the OpenBSD community would probably prefer (or at least not be upset) that it not be used even in versions of OpenBSD that have the syscall.

@aantron
Copy link
Collaborator

aantron commented Feb 21, 2019

Thanks.

I would also argue that mincore() has been determined to be a security problem with no counterbalancing advantage, and the OpenBSD community would probably prefer (or at least not be upset) that it not be used even in versions of OpenBSD that have the syscall.

This should remain between the OpenBSD developers and the OpenBSD users. Lwt shouldn't implicitly insert its own judgment here by eagerly disallowing it, especially if that would be a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants