tools/ocaml: oxenstored: Be more paranoid about ring reading
authorIan Jackson <ian.jackson@eu.citrix.com>
Thu, 7 Feb 2013 14:23:56 +0000 (14:23 +0000)
committerIan Jackson <ian.jackson@eu.citrix.com>
Thu, 7 Feb 2013 14:23:56 +0000 (14:23 +0000)
commit40f9c5e0a6d15b4ca1f6d4ed3a46f0871520eab5
tree2d739967c5b3d4f86acbf5c1badb9994a743c451
parent94be624294ab711528ac0eddca3259adf65669b4
tools/ocaml: oxenstored: Be more paranoid about ring reading

oxenstored makes use of the OCaml Xenbus bindings, in which the
function xs_ring_read in tools/ocaml/libs/xb/xs_ring_stubs.c is used
to read from the shared memory Xenstore ring.

This function does not correctly handle all possible (prod, cons)
states when MASK_XENSTORE_IDX(prod) > MASK_XENSTORE_IDX(cons).

The root cause is the use of the unmasked values of prod and cons to
calculate to_read.  If prod is set to an out-of-range value, the ring
peer can cause to_read to be too large or even negative.  This allows
the ring peer to force oxenstored to read and write out of range for
the buffers leading to a crash or possibly to privilege escalation.

Correct this by masking the values of cons and prod at the start, so
we only deal with masked values.  This makes the logic simpler, as
semantically inappropriate values of the upper bits of the ring
pointers are simply ignored.

The same vulnerability does not exist in the ring writer because the
only use made of the unmasked value is the check which prevents the
prod pointer overtaking the cons pointer.  A ring peer which defeats
this check will suffer only lost data.

However, additionally, precautions need to be taken to ensure that
req_cons and req_prod are only read once in each function.  Without
the use of volatile or some asm construct, the compiler can "prove"
that req_cons and req_prod do not change unexpectedly and is permitted
to "amplify" the read of (say) req_cons into two reads at different
times, giving two different values for use as cons, and then use the
two sources of cons interchangeably.  (The use of xen_mb() does not
forbid this.)

Therefore do the reads of req_cons and req_prod through a volatile
pointer in both xs_ring_read and xs_ring_write.

This is currently believed to be a theoretical vulnerability as we are
not aware of any compilers which amplify reads in this way.

This is a security issue, part of XSA-38 / CVE-2013-0215.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Matthew Daley <mattjd@gmail.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset: 26521:2c0fd406f02c
Backport-requested-by: security@xen.org
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
tools/ocaml/libs/xb/xs_ring_stubs.c