Re: [Xen-devel] [PATCH 1/2] xenstore: sanity check incoming message body lengths [and 2 more messages]

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-12-19 at 15:42 +0000, Ian Jackson wrote:
> Matthew Daley writes ("[PATCH 1/2] xenstore: sanity check incoming message 
> body lengths"):
> > This is for the client-side receiving messages from xenstored, so there
> > is no security impact, unlike XSA-72.
> 
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH 1/2] xenstore: sanity check 
> incoming message body lengths"):
> > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Matthew Daley writes ("Re: [PATCH 1/2] xenstore: sanity check incoming 
> message body lengths"):
> > On Tue, Dec 3, 2013 at 12:33 AM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> 
> > wrote:
> > > If this situation should arise, your proposal would discard the
> > > headers of the bogus message and read the start of what would be the
> > > over-long payload as the next header.
> ...
> > > It would be best to either kill the connection dead, or perhaps to
> > > skip the overlong data.
> > 
> > The only callers of read_message I can see are read_reply,
> > read_watch_internal and read_thread. read_reply's only caller is
> > xs_talkv, which closes the connection on the failure being passed
> > down. read_watch_internal doesn't, and neither do its callers.
> > read_thread does close the connection.
> > 
> > So, with that said, where should the handling of the failure go? Would
> > it be good to consolidate the handling in one spot, ie. directly in
> > read_message? Since read_message is the root function for all these
> > other functions, and since read_thread already does handle the failure
> > and the other functions use that if implicitly if threaded background
> > reading is enabled, it seems like a good idea to do so.
> 
> Yes.
> 
> Arguably, based on what you've said, this patch should be applied
> as-is, on the grounds of it being an improvement.  Ian C, do you have
> an opinion ?

I don't see the harm in taking this, it is an improvement.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel