[dm-devel] [PATCH] libmultipath: fix memory API logic error

[Johannes Thumshirn]

On Mon, Aug 15, 2016 at 06:59:09PM +0800, tang.junhui at zte.com.cn wrote:
> From: "tang.junhui" <tang.junhui at zte.com.cn>
> 
> Memroy API use mem_allocated to record the total size of used memory,
> however, it's wrong to use size(p) as the length of freed memory in xfree(),
> and memory may also be allocated by STRDUP() or REALLOC(), which is
> not calculated into mem_allocated, so the total size of used memory is
> not correctly. For these reasons, we removed these incorrectly code to keep
> the code clean.
> 
> Signed-off-by: tang.junhui <tang.junhui at zte.com.cn>
> ---
>  libmpathpersist/mpath_updatepr.c |  1 -
>  libmultipath/memory.c            | 31 -------------------------------
>  libmultipath/memory.h            | 13 ++-----------
>  3 files changed, 2 insertions(+), 43 deletions(-)
> 
> diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c
> index 9ff4b30..5af2e03 100644
> --- a/libmpathpersist/mpath_updatepr.c
> +++ b/libmpathpersist/mpath_updatepr.c
> @@ -16,7 +16,6 @@
>  #include "uxsock.h"
>  #include "memory.h"
>  
> -unsigned long mem_allocated;    /* Total memory used in Bytes */
>  
>  int update_prflag(char * arg1, char * arg2, int noisy)
>  {
> diff --git a/libmultipath/memory.c b/libmultipath/memory.c
> index 1366f45..5441e6a 100644
> --- a/libmultipath/memory.c
> +++ b/libmultipath/memory.c
> @@ -22,37 +22,6 @@
>

[...]

> -
> -void
> -xfree(void *p)
> -{
> -	mem_allocated -= sizeof (p);
> -	free(p);
> -	p = NULL;
> -}
> -
>  /*

[...]

> -#define MALLOC(n)    (zalloc(n))
> -#define FREE(p)      (xfree(p))
> +#define MALLOC(n)    (calloc(1,(n)))
> +#define FREE(p)      (free(p))
>  #define REALLOC(p,n) (realloc((p),(n)))
>  #define STRDUP(n)    (strdup(n))

One minor nit, the original xfree() has set the freed pointer to 0, so maybe a

#define FREE(p) do { free(p); p = NULL } while(0)

would be advisable. I personally like setting free'd pointers to NULL,
as a NULL pointer access is a more immediate feedback than some rather
nasty use-after-free.

Just my $0.2

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850