Handle potential invalid pointer reference for VMCI reply. #973
Conversation
| @@ -291,6 +291,7 @@ vsock_get_reply(be_sock_id *s, be_request *r, be_answer* a) | |||
| ret = recv(s->sock_id, a->buf, b, 0); | |||
| if (ret == -1 || ret != b) { | |||
| free(a->buf); | |||
| a->buf = NULL; | |||
There was a problem hiding this comment.
nit - well, just to be paranoid, it should be
if (a->buf) {
free(a->buf);
a->buf = NULL;
}I know a->buf is not null (per L286-L289, but just to keep this section easier to read and more resilient to changes above it :-)
| @@ -349,5 +350,7 @@ Vmci_GetReply(int port, const char* json_request, const char* be_name, | |||
| req.mlen = strnlen(json_request, MAXBUF) + 1; | |||
| req.msg = json_request; | |||
|
|
|||
| ans->buf = NULL; | |||
There was a problem hiding this comment.
wait, why here? if you want it to be NULL, the right place to change would be changing C.Malloc() to C.Calloc() in esx_vmdkcmd.go.
| @@ -117,6 +117,11 @@ func (vmdkCmd EsxVmdkCmd) Run(cmd string, name string, opts map[string]string) ( | |||
| break | |||
| } | |||
|
|
|||
| if ans.buf == nil { | |||
There was a problem hiding this comment.
I do no think we can get here in failure case without printing ANY message from the loop above, and the logs did not have any message, so I do not see why this block is needed. Besides, with the changes suggested in this PR this can be impossible, will it ? Besides, in cgo is it ok to compare go.nil with C (unsafe) ans.buf ?
- if you want to for working on a repro, this makes sense, but for production code it feels not needed
|
The changes are being tracked via #941, comments are addressed and closing this PR. |
Added a check for NULL recv. buffer in the Esx_VmdkCmd:Run() method and in VMCI client code added setting the buf pointer to NULL on entry and on recv failures.