Re: [PATCH] XEN: xenbus: integer overflow in process_msg()

[Ian Campbell]

On Wed, 2012-01-04 at 09:24 +0000, Ian Campbell wrote:
> On Tue, 2012-01-03 at 19:42 +0000, Haogang Chen wrote:
> > There is a potential integer overflow in process_msg() that could result
> > in cross-domain attack.
> > 
> > 	body = kmalloc(msg->hdr.len + 1, GFP_NOIO | __GFP_HIGH);
> > 
> > When a malicious guest passes 0xffffffff in msg->hdr.len, the subsequent
> > call to xb_read() would write to a zero-length buffer.
> 
> The other end of this connection is always the xenstore backend daemon
> so there is no guest (malicious or otherwise) which can do this. The
> xenstore daemon is a trusted component in the system.
> 
> However this seem like a reasonable robustness improvement so we should
> have it.

Actually, rereading docs/misc/xenstore.txt I see:
        The payload length (len field of the header) is limited to 4096
        (XENSTORE_PAYLOAD_MAX) in both directions.  If a client exceeds the
        limit, its xenstored connection will be immediately killed by
        xenstored, which is usually catastrophic from the client's point of
        view.  Clients (particularly domains, which cannot just reconnect)
        should avoid this.

So probably we actually want (untested, but seems obvious enough):

8<---------------------------------------------------------