Re: [PATCH] oxenstored: fix short-write issue

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/10/15 17:10, Wei Liu wrote:
> When oxenstored wrote to the ring, it wrote a chunk of contiguous data.
> Originally when it tried to write across ring boundary, it returned a
> short-write when there is still room.  That led to stalling mini-os's
> xenstore thread at times.
>
> Fix this by calling write function for a second time when the first
> write completes partially.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: David Scott <dave@recoil.org>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>
> Ian, backport candidate for as far as you can manage.
> ---
>  tools/ocaml/libs/xb/xb.ml | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml
> index 50944b5..0730d13 100644
> --- a/tools/ocaml/libs/xb/xb.ml
> +++ b/tools/ocaml/libs/xb/xb.ml
> @@ -91,10 +91,12 @@ let write_fd back con s len =
>  	Unix.write back.fd s 0 len
>  
>  let write_mmap back con s len =
> -	let ws = Xs_ring.write back.mmap s len in
> -	if ws > 0 then
> +	let ws = ref (Xs_ring.write back.mmap s len) in
> +	if !ws < len then
> +		ws := !ws + Xs_ring.write back.mmap (String.sub s !ws (len - !ws)) (len - !ws);

This is surely a TOCTOU, as the second attempted write could return
short as well.

The correct behaviour would be for Xs_ring.write to return the actual
number of bytes it put into the ring, even if this is shorter than len.

~Andrew