[libvirt] [PATCH] util: fix heap-buffer-overflow in virFileWrapperFdFree

[Ján Tomko]

On Tue, Feb 19, 2019 at 03:40:31PM +0800, Jay Zhou wrote:
>From: Jing Wu <wujing42 at huawei.com>
>
>Some functions like doCoreDump call virFileWrapperFdNew to execute async
>cmds, if a step after virFileWrapperFdNew failed, the func may skip
>virFileWrapperFdClose and jump to cleanup label to call
>virFileWrapperFdFree directly.

Wouldn't the proper fix be to always call virFileWrapperFdClose?
See this series by Andrea:
https://www.redhat.com/archives/libvir-list/2019-February/msg01155.html

>If the child process of the cmd is running
>and asyncioThread is polling, cmd->errbuf have been alloced at least one
>byte but not yet operate (*cmd->errbuf)[errlen] = '\0', access of
>wfd->err_msg in virFileWrapperFdFree at this time will cause risk of
>heap-buffer-overflow.

>So, we need to put VIR_WARN(wfd->err_msg) after VIR_FREE(wfd->err_msg).

This sentence does not make sense - why would we access the error after
freeing it?

Jano

>Besides, since virCommandFree has included virCommandAbort, there is no
>need to call virCommandAbort extraly.
>
>Signed-off-by: Jing Wu <wujing42 at huawei.com>
>Signed-off-by: Jay Zhou <jianjay.zhou at huawei.com>
>---
> src/util/virfile.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190225/50922b3f/attachment-0001.sig>