On Thu, Dec 17, 2020 at 12:00:13PM +0100, [email protected] wrote: > From: Martin Wilck <[email protected]> > > 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 <[email protected]> > --- > 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 <[email protected]> > @@ -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 [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
