Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004839OCamlOCaml generalpublic2009-07-22 08:152016-02-03 12:21
Reporterfuruse 
Assigned To 
PrioritynormalSeveritycrashReproducibilityalways
StatusacknowledgedResolutionopen 
PlatformOSOS Version
Product Version3.11.1 
Target VersionlaterFixed in Version 
Summary0004839: natdynlink reproducible segfault
DescriptionA 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.
TagsNo tags attached.
Attached Filestgz file icon natdynlink-crash.tgz [^] (1,666 bytes) 2009-07-22 08:15
patch file icon fail.patch [^] (635 bytes) 2016-01-15 21:07 [Show Content]
patch file icon ignore.patch [^] (586 bytes) 2016-01-15 21:07 [Show Content]
patch file icon safe.patch [^] (917 bytes) 2016-01-15 23:00 [Show Content]

- Relationships
related to 0004231closedlefessan Link multiple casse le système de types 
related to 0004229acknowledged Casser le typage avec Dynlink 
related to 0006462acknowledged Dynlinking duplicate module clobbers host program state 
related to 0006957acknowledged Modules name conflict during dynamic loading 
related to 0006950acknowledged Dynlink wrong symbol 

-  Notes
(0005028)
frisch (developer)
2009-07-22 09:18

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.
(0014867)
frisch (developer)
2015-11-27 16:56

Turning to later: no clear resolution plan, and bug present for years.
(0014876)
xleroy (administrator)
2015-11-28 10:24

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.
(0015258)
ivg (reporter)
2016-01-15 21:27
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?

(0015259)
ivg (reporter)
2016-01-15 23:03

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.
(0015260)
glondu (reporter)
2016-01-18 11:50

> 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.

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.
(0015263)
ivg (reporter)
2016-01-18 18:05

> 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.

- Issue History
Date Modified Username Field Change
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


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker