On Thu, Dec 17, 2020 at 12:00:13PM +0100, mwilck@xxxxxxxx wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> Make sure the global logarea (la) is only allocated and freed
> hile holding logq_lock. This avoids invalid memory access.
>
> This patch makes free_logarea() static. libmultipath.version
> is unchanged, as free_logarea() wasn't exported anyway.
>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> libmultipath/log.c | 32 +++++++++++++++++++++++---------
> libmultipath/log.h | 1 -
> 2 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/libmultipath/log.c b/libmultipath/log.c
> index 7f33787..95c8f01 100644
> --- a/libmultipath/log.c
> +++ b/libmultipath/log.c
> @@ -77,16 +77,23 @@ static int logarea_init (int size)
>
> int log_init(char *program_name, int size)
> {
> + int ret = 1;
> +
> logdbg(stderr,"enter log_init\n");
> +
> + pthread_mutex_lock(&logq_lock);
> + pthread_cleanup_push(cleanup_mutex, &logq_lock);
> +
> openlog(program_name, 0, LOG_DAEMON);
> + if (!la)
> + ret = logarea_init(size);
>
> - if (logarea_init(size))
> - return 1;
> + pthread_cleanup_pop(1);
>
> - return 0;
> + return ret;
> }
>
> -void free_logarea (void)
> +static void free_logarea (void)
> {
> FREE(la->start);
> FREE(la->buff);
I realize that the log area can only be freed by log_close(), which is
called when mulitpathd exits, but it would be nice to have la set to
NULL it's freed, just to make it obvious that that there can't be
double-frees there. However, the code is clearly correct, so
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> @@ -96,9 +103,14 @@ void free_logarea (void)
>
> void log_close (void)
> {
> - free_logarea();
> + pthread_mutex_lock(&logq_lock);
> + pthread_cleanup_push(cleanup_mutex, &logq_lock);
> +
> + if (la)
> + free_logarea();
> closelog();
>
> + pthread_cleanup_pop(1);
> return;
> }
>
> @@ -175,11 +187,12 @@ static int _log_enqueue(int prio, const char * fmt, va_list ap)
>
> int log_enqueue(int prio, const char *fmt, va_list ap)
> {
> - int ret;
> + int ret = 1;
>
> pthread_mutex_lock(&logq_lock);
> pthread_cleanup_push(cleanup_mutex, &logq_lock);
> - ret = _log_enqueue(prio, fmt, ap);
> + if (la)
> + ret = _log_enqueue(prio, fmt, ap);
> pthread_cleanup_pop(1);
> return ret;
> }
> @@ -215,11 +228,12 @@ static int _log_dequeue(void *buff)
>
> int log_dequeue(void *buff)
> {
> - int ret;
> + int ret = 1;
>
> pthread_mutex_lock(&logq_lock);
> pthread_cleanup_push(cleanup_mutex, &logq_lock);
> - ret = _log_dequeue(buff);
> + if (la)
> + ret = _log_dequeue(buff);
> pthread_cleanup_pop(1);
> return ret;
> }
> diff --git a/libmultipath/log.h b/libmultipath/log.h
> index d2448f6..fa224e4 100644
> --- a/libmultipath/log.h
> +++ b/libmultipath/log.h
> @@ -39,6 +39,5 @@ int log_enqueue (int prio, const char * fmt, va_list ap)
> int log_dequeue (void *);
> void log_syslog (void *);
> void dump_logmsg (void *);
> -void free_logarea (void);
>
> #endif /* LOG_H */
> --
> 2.29.0
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
