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.coop...@citrix.com>
>> Coverity-IDs: 1055305 1055721
>> CC: Ian Campbell <ian.campb...@citrix.com>
>> CC: Ian Jackson <ian.jack...@eu.citrix.com>
>> CC: Wei Liu <wei.l...@citrix.com>
>> CC: Xen Coverity Team <cover...@xen.org>
> Acked-by: Ian Campbell <ian.campb...@citrix.com>
>
> 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.
One solution to this was strdup(), but as the duplication was
unnecessary anyway, it was easier just to drop it all.
~Andrew
>
>> ---
>> tools/python/xen/lowlevel/xc/xc.c | 21 +++------------------
>> 1 file changed, 3 insertions(+), 18 deletions(-)
>>
>> diff --git a/tools/python/xen/lowlevel/xc/xc.c
>> b/tools/python/xen/lowlevel/xc/xc.c
>> index d95d459..c70b388 100644
>> --- a/tools/python/xen/lowlevel/xc/xc.c
>> +++ b/tools/python/xen/lowlevel/xc/xc.c
>> @@ -2126,8 +2126,6 @@ static PyObject *pyflask_context_to_sid(PyObject
>> *self, PyObject *args,
>> {
>> xc_interface *xc_handle;
>> char *ctx;
>> - char *buf;
>> - uint32_t len;
>> uint32_t sid;
>> int ret;
>>
>> @@ -2137,28 +2135,15 @@ static PyObject *pyflask_context_to_sid(PyObject
>> *self, PyObject *args,
>> &ctx) )
>> return NULL;
>>
>> - len = strlen(ctx);
>> -
>> - buf = malloc(len);
>> - if (!buf) {
>> - errno = -ENOMEM;
>> - PyErr_SetFromErrno(xc_error_obj);
>> - }
>> -
>> - memcpy(buf, ctx, len);
>> -
>> xc_handle = xc_interface_open(0,0,0);
>> if (!xc_handle) {
>> - free(buf);
>> return PyErr_SetFromErrno(xc_error_obj);
>> }
>> -
>> - ret = xc_flask_context_to_sid(xc_handle, buf, len, &sid);
>> -
>> +
>> + ret = xc_flask_context_to_sid(xc_handle, ctx, strlen(ctx), &sid);
>> +
>> xc_interface_close(xc_handle);
>>
>> - free(buf);
>> -
>> if ( ret != 0 ) {
>> errno = -ret;
>> return PyErr_SetFromErrno(xc_error_obj);
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
