Re: [Xen-devel] [PATCH for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-11-28 at 11:47 +0000, Andrew Cooper wrote:
> On 28/11/14 11:37, Ian Campbell wrote:
> > On Thu, 2014-11-27 at 12:34 +0000, Andrew Cooper wrote:
> >> The error handling from a failed memory allocation should return
> >> PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and 
> >> continuing
> >> to the memcpy() below, with the dest pointer being NULL.
> >>
> >> Furthermore, the context string is simply an input parameter to the 
> >> hypercall,
> >> and is not mutated anywhere along the way.  The error handling elsewhere in
> >> the function can be simplified by not duplicating it to start with.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> Coverity-IDs: 1055305 1055721
> >> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> >> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> >> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> CC: Xen Coverity Team <coverity@xxxxxxx>
> > Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> >
> > This would have been far more obviously correct for 4.5 if you had stuck
> > to fixing the issue in the first paragraph.
> 
> Hmm - I appear to have missed a detail in the description.  One of the
> CIDs is to do with putting a string in a new buffer without a NUL
> terminator, and passing it as a char* into xc_flask_context_to_sid; the
> issue being that anyone expecting things like strlen() to work will be
> in for a nasty shock.

ISTR a discussion of this interface in Julien's device-tree passthrough
thing a while back and some sort of conclusion that the hypervisor was
supposed to use the len field and not rely on the NULL.

I'm not sure that necessarily invalidates what you are saying, since
even given that throwing a NULL on the end would be friendly to libxc
consumers if nothing else.

Ian.



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