# HG changeset patch
# User Zheng Li <dev@xxxxxxxx>
# Date 1317919543 -3600
# Node ID 4d4edbc963560a25547eb91ffaec1ec273b09b38
# Parent 4c4b72c94bac8d14b5046b80ed5569c8397d0de1
tools: ocaml: Fix invalid memory access in OCaml mmap library
Fix invalid memory access in OCaml mmap library (to play nicely with the GC)
This was a bug reported by Roberto Di Cosmo. When he tried to reuse
the mmap library for his own project, Mmap.read occasionally got
different result when reading from the same map. This turned out to be
a bug in the binding, where a C pointer was created pointing to a
OCaml value, and the OCaml value was subsequently moved around by the
GC after memory allocation and hence invalidated the C pointer. This
patch removes the indirection of C pointer and uses OCaml macro to
access values directly.
Only Mmap.read function had this problem. The other functions, despite
having the same code style, didn't have memory allocation involved
hence wouldn't intrigue such an error. I've changed all of them to the
safer style for future proof. Directly casting OCaml value's *data
block* (rather than the value itself) as a C pointer is not a common
practice either, but I'll leave it as it is.
The bug hadn't occured on XenServer because XenServer didn't make use
of the Mmap.read function (except in one place for debugging). In
XenServer, most mmap operations were going through another pair of
separately implemented functions (Xs_ring.read/write).
Signed-off-by: Zheng Li <dev@xxxxxxxx>
Committed-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
diff -r 4c4b72c94bac -r 4d4edbc96356 tools/ocaml/libs/mmap/mmap_stubs.c
--- a/tools/ocaml/libs/mmap/mmap_stubs.c Thu Oct 06 17:26:18 2011 +0100
+++ b/tools/ocaml/libs/mmap/mmap_stubs.c Thu Oct 06 17:45:43 2011 +0100
@@ -71,12 +71,10 @@
CAMLprim value stub_mmap_final(value interface)
{
CAMLparam1(interface);
- struct mmap_interface *intf;
- intf = GET_C_STRUCT(interface);
- if (intf->addr != MAP_FAILED)
- munmap(intf->addr, intf->len);
- intf->addr = MAP_FAILED;
+ if (GET_C_STRUCT(interface)->addr != MAP_FAILED)
+ munmap(GET_C_STRUCT(interface)->addr,
GET_C_STRUCT(interface)->len);
+ GET_C_STRUCT(interface)->addr = MAP_FAILED;
CAMLreturn(Val_unit);
}
@@ -85,21 +83,19 @@
{
CAMLparam3(interface, start, len);
CAMLlocal1(data);
- struct mmap_interface *intf;
int c_start;
int c_len;
c_start = Int_val(start);
c_len = Int_val(len);
- intf = GET_C_STRUCT(interface);
- if (c_start > intf->len)
+ if (c_start > GET_C_STRUCT(interface)->len)
caml_invalid_argument("start invalid");
- if (c_start + c_len > intf->len)
+ if (c_start + c_len > GET_C_STRUCT(interface)->len)
caml_invalid_argument("len invalid");
data = caml_alloc_string(c_len);
- memcpy((char *) data, intf->addr + c_start, c_len);
+ memcpy((char *) data, GET_C_STRUCT(interface)->addr + c_start, c_len);
CAMLreturn(data);
}
@@ -108,20 +104,18 @@
value start, value len)
{
CAMLparam4(interface, data, start, len);
- struct mmap_interface *intf;
int c_start;
int c_len;
c_start = Int_val(start);
c_len = Int_val(len);
- intf = GET_C_STRUCT(interface);
- if (c_start > intf->len)
+ if (c_start > GET_C_STRUCT(interface)->len)
caml_invalid_argument("start invalid");
- if (c_start + c_len > intf->len)
+ if (c_start + c_len > GET_C_STRUCT(interface)->len)
caml_invalid_argument("len invalid");
- memcpy(intf->addr + c_start, (char *) data, c_len);
+ memcpy(GET_C_STRUCT(interface)->addr + c_start, (char *) data, c_len);
CAMLreturn(Val_unit);
}
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home