|Anonymous | Login | Signup for a new account||2017-10-21 17:56 CEST|
|Main | My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0004839||OCaml||otherlibs||public||2009-07-22 08:15||2017-06-09 17:17|
|Target Version||later||Fixed in Version|
|Summary||0004839: natdynlink reproducible segfault|
|Description||A packed module in a dll cmxs overwrites the packed module of the same name in the host program at dynlinking. If these two packed modules have incompatible signatures, the program may easily seg-faults.|
We have found this when we wrongly created dll cmxs linking with a module of the host program. I upload a reproducible example. make all test shows the crash.
- plugin : a base dll which works fine
- plugin2 : a dll with a incompatible signature. Rejected correctly at linking.
- plugin3 : same as plugin2, but mistakenly linked with the api module. Self-contained. Crashes the host program at linking.
- plugin4 : a dll which shows that the overwrite. The host programs Packed.Api.zero is overwritten by the dll's Packed.Api.zero.
|Tags||No tags attached.|
|Attached Files|| natdynlink-crash.tgz [^] (1,666 bytes) 2009-07-22 08:15|
fail.patch [^] (635 bytes) 2016-01-15 21:07 [Show Content]
ignore.patch [^] (586 bytes) 2016-01-15 21:07 [Show Content]
safe.patch [^] (917 bytes) 2016-01-15 23:00 [Show Content]
|I reported such a problem some time ago: 0004229. In fact, you don't even need to use dynlink in order to break the type system when you link in different modules with the same name/signature; regular packing/linking will do: 0004231.|
|Turning to later: no clear resolution plan, and bug present for years.|
|Just refusing to dynlink a compilation unit A if the main program already contains a unit named A would go a long way towards fixing this group of PRs.|
edited on: 2016-01-15 22:16
I've uploaded two solutions, the first one is to bail out if someone tries to reinitialize a unit that is already in the main program. (See fail.patch)
Another approach, that is less conservative, and requires some consideration is to silently ignore such units and keep moving. (See ignore.patch).
I would advocate for the latter solution. First of all it looks safe, as we check the module that we ignore for a consistency with that one, that is already loaded. Second, it has intuitive behavior, i.e., it is ok to link two programs, that imports the same library. Third ... if we will bail out, then it may break plugin solutions for BAP, Frama-C, and maybe other projects. But honestly speaking, I still can't proof to myself, that this is a safe solution.
As far as I understand, the root of the problem here is that whenever we create a plugin that references some library as `cmx` or `cmxa` then the definitions (ui_defines) from the referenced library are copied to the `dynu_defines` field of the plugin. As a result the code is included, instead of being imported. So, ideal solution would be to never include other libraries in the plugin (do not use `-linkpkg` in ocamlfind's parlance). Then we will always work with a thin plugin, i.e., a shared library that never includes any other libraries, but only imports them. And at load time, we should satisfy all imports, that are not yet linked into the host program.
P.S. I'm not sure what Xavier meant by "refuse". To raise an error or to silently ignore?
|I've upload a `safe.patch` that looks more safe for me than the `ignore.patch`, as it explicitly checks for all global names. This should also work for packaged objects.|
> So, ideal solution would be to never include other libraries in the
> plugin (do not use `-linkpkg` in ocamlfind's parlance). Then we will
> always work with a thin plugin, i.e., a shared library that never
> includes any other libraries, but only imports them. And at load time,
> we should satisfy all imports, that are not yet linked into the host
This is what should be done in my humble opinion.
In Ocsigen, we create a Findlib package per plugin, and use Findlib in the host program to figure out what cmxs need to be loaded at dynlink time.
> In Ocsigen, we create a Findlib package per plugin, and use Findlib in the host program to
> figure out what cmxs need to be loaded at dynlink time.
Yep, we're using the same approach. But findlib even with its new dynload module, that records already loaded dependencies and predicates, still can't protect you fully from a corruption of state and segfaults. The problem is that findlib doesn't have an access to an internal `caml_plugin_header` data, so it can track dependencies only on its own level of abstraction, that not necessary reflects the real situation. For example, oasis will break this abstractions and link internal libraries without using `-package` option.
|This has been fixed by https://github.com/ocaml/ocaml/pull/1063 [^] - please continue any discussion there.|
|2009-07-22 08:15||furuse||New Issue|
|2009-07-22 08:15||furuse||File Added: natdynlink-crash.tgz|
|2009-07-22 09:18||frisch||Note Added: 0005028|
|2009-07-22 09:18||frisch||Relationship added||related to 0004231|
|2009-07-22 09:19||frisch||Relationship added||parent of 0004229|
|2009-07-22 09:19||frisch||Relationship deleted||parent of 0004229|
|2009-07-22 09:19||frisch||Relationship added||related to 0004229|
|2010-04-21 11:34||doligez||Status||new => acknowledged|
|2012-07-06 16:08||doligez||Target Version||=> 4.01.0+dev|
|2012-07-31 13:36||doligez||Target Version||4.01.0+dev => 4.00.1+dev|
|2012-09-06 19:12||frisch||Target Version||4.00.1+dev => 4.00.2+dev|
|2013-06-03 16:48||doligez||Target Version||4.00.2+dev => 4.02.0+dev|
|2013-07-12 18:15||doligez||Target Version||4.02.0+dev => 4.01.1+dev|
|2014-05-25 20:20||doligez||Target Version||4.01.1+dev => 4.02.0+dev|
|2014-07-16 10:22||doligez||Relationship added||related to 0006462|
|2014-07-16 20:07||doligez||Target Version||4.02.0+dev => 4.02.1+dev|
|2014-09-04 00:25||doligez||Target Version||4.02.1+dev => undecided|
|2014-09-26 17:42||doligez||Target Version||undecided => 4.03.0+dev / +beta1|
|2015-11-20 18:10||xleroy||Relationship added||related to 0006957|
|2015-11-27 16:56||frisch||Note Added: 0014867|
|2015-11-27 16:56||frisch||Target Version||4.03.0+dev / +beta1 => later|
|2015-11-28 10:24||xleroy||Note Added: 0014876|
|2016-01-15 21:07||ivg||File Added: fail.patch|
|2016-01-15 21:07||ivg||File Added: ignore.patch|
|2016-01-15 21:27||ivg||Note Added: 0015258|
|2016-01-15 22:16||ivg||Note Edited: 0015258||View Revisions|
|2016-01-15 23:00||ivg||File Added: safe.patch|
|2016-01-15 23:03||ivg||Note Added: 0015259|
|2016-01-18 11:50||glondu||Note Added: 0015260|
|2016-01-18 18:05||ivg||Note Added: 0015263|
|2016-02-03 12:21||doligez||Relationship added||related to 0006950|
|2016-12-08 13:20||shinwell||Category||OCaml general => OCaml otherlibs|
|2017-02-23 16:42||doligez||Category||OCaml otherlibs => otherlibs|
|2017-02-24 14:29||shinwell||Assigned To||=> shinwell|
|2017-02-24 14:29||shinwell||Status||acknowledged => assigned|
|2017-06-09 17:17||shinwell||Note Added: 0017858|
|2017-06-09 17:17||shinwell||Status||assigned => resolved|
|2017-06-09 17:17||shinwell||Resolution||open => duplicate|
|Copyright © 2000 - 2011 MantisBT Group|