[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 05/24] maint: don't leave garbage on early API exit




On 12/28/2013 11:11 AM, Eric Blake wrote:
> Several APIs clear out a user input buffer before attempting to
> populate it; but in a few cases we missed this memset if we
> detect a reason for an early exit.
> 
> * src/libvirt.c (virDomainGetInfo, virDomainGetBlockInfo)
> (virStoragePoolGetInfo, virStorageVolGetInfo)
> (virDomainGetJobInfo, virDomainGetBlockJobInfo): Move memset
> before any returns.
> 
> Signed-off-by: Eric Blake <eblake redhat com>
> ---
>  src/libvirt.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 

Not sure any of these are correct... Each of the clear outs is after a
check of whether the passed 'info' buffer is non NULL.

For each that means you'd potentially have "memset(NULL, 0, sizeof())"

John
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 33f7078..7fd6072 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -4461,6 +4461,8 @@ virDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
> 
>      virResetLastError();
> 
> +    memset(info, 0, sizeof(virDomainInfo));
> +
>      if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
>          virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
>          virDispatchError(NULL);
> @@ -4468,8 +4470,6 @@ virDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>      }
>      virCheckNonNullArgGoto(info, error);
> 
> -    memset(info, 0, sizeof(virDomainInfo));
> -
>      conn = domain->conn;
> 
>      if (conn->driver->domainGetInfo) {
> @@ -9114,6 +9114,8 @@ virDomainGetBlockInfo(virDomainPtr domain, const char *disk,
> 
>      virResetLastError();
> 
> +    memset(info, 0, sizeof(virDomainBlockInfo));
> +
>      if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
>          virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
>          virDispatchError(NULL);
> @@ -9122,8 +9124,6 @@ virDomainGetBlockInfo(virDomainPtr domain, const char *disk,
>      virCheckNonNullArgGoto(disk, error);
>      virCheckNonNullArgGoto(info, error);
> 
> -    memset(info, 0, sizeof(virDomainBlockInfo));
> -
>      conn = domain->conn;
> 
>      if (conn->driver->domainGetBlockInfo) {
> @@ -14297,6 +14297,8 @@ virStoragePoolGetInfo(virStoragePoolPtr pool,
> 
>      virResetLastError();
> 
> +    memset(info, 0, sizeof(virStoragePoolInfo));
> +
>      if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) {
>          virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__);
>          virDispatchError(NULL);
> @@ -14304,8 +14306,6 @@ virStoragePoolGetInfo(virStoragePoolPtr pool,
>      }
>      virCheckNonNullArgGoto(info, error);
> 
> -    memset(info, 0, sizeof(virStoragePoolInfo));
> -
>      conn = pool->conn;
> 
>      if (conn->storageDriver->storagePoolGetInfo) {
> @@ -15281,6 +15281,8 @@ virStorageVolGetInfo(virStorageVolPtr vol,
> 
>      virResetLastError();
> 
> +    memset(info, 0, sizeof(virStorageVolInfo));
> +
>      if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) {
>          virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
>          virDispatchError(NULL);
> @@ -15288,8 +15290,6 @@ virStorageVolGetInfo(virStorageVolPtr vol,
>      }
>      virCheckNonNullArgGoto(info, error);
> 
> -    memset(info, 0, sizeof(virStorageVolInfo));
> -
>      conn = vol->conn;
> 
>      if (conn->storageDriver->storageVolGetInfo){
> @@ -18918,6 +18918,8 @@ virDomainGetJobInfo(virDomainPtr domain, virDomainJobInfoPtr info)
> 
>      virResetLastError();
> 
> +    memset(info, 0, sizeof(virDomainJobInfo));
> +
>      if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
>          virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
>          virDispatchError(NULL);
> @@ -18925,8 +18927,6 @@ virDomainGetJobInfo(virDomainPtr domain, virDomainJobInfoPtr info)
>      }
>      virCheckNonNullArgGoto(info, error);
> 
> -    memset(info, 0, sizeof(virDomainJobInfo));
> -
>      conn = domain->conn;
> 
>      if (conn->driver->domainGetJobInfo) {
> @@ -21245,6 +21245,8 @@ virDomainGetBlockJobInfo(virDomainPtr dom, const char *disk,
> 
>      virResetLastError();
> 
> +    memset(info, 0, sizeof(*info));
> +
>      if (!VIR_IS_CONNECTED_DOMAIN(dom)) {
>          virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
>          virDispatchError(NULL);
> @@ -21255,8 +21257,6 @@ virDomainGetBlockJobInfo(virDomainPtr dom, const char *disk,
>      virCheckNonNullArgGoto(disk, error);
>      virCheckNonNullArgGoto(info, error);
> 
> -    memset(info, 0, sizeof(*info));
> -
>      if (conn->driver->domainGetBlockJobInfo) {
>          int ret;
>          ret = conn->driver->domainGetBlockJobInfo(dom, disk, info, flags);
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]