From 590a7718c948d02ad93ce4f68c9c75ec0674a3cd Mon Sep 17 00:00:00 2001 From: Danny Couture Date: Wed, 5 Jul 2017 09:55:17 -0400 Subject: fuse: memory leak fixes Fix fuse ctx memory leak in case an error occurs and the cleanup path is different than usual. Also fix a memory leak in logging if eh_save_history() fails. Cherry picked from commit 5ee383fed9f6408d303aa539dda071275021f8e4: > Change-Id: I7ec967c807b0ed91184e5b958be70702215c46c9 > BUG: 1470220 > Signed-off-by: Danny Couture > Reviewed-on: https://review.gluster.org/17759 > Reviewed-by: Niels de Vos > Smoke: Gluster Build System > Reviewed-by: N Balachandran > Reviewed-by: Prashanth Pai > Reviewed-by: Amar Tumballi > Tested-by: Amar Tumballi > CentOS-regression: Gluster Build System > Reviewed-by: Raghavendra G Change-Id: I7ec967c807b0ed91184e5b958be70702215c46c9 BUG: 1471025 Signed-off-by: Niels de Vos Reviewed-on: https://review.gluster.org/17775 Smoke: Gluster Build System CentOS-regression: Gluster Build System --- libglusterfs/src/logging.c | 2 ++ xlators/mount/fuse/src/fuse-bridge.c | 68 ++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/libglusterfs/src/logging.c b/libglusterfs/src/logging.c index a8e7b96a24c..3436b9f4989 100644 --- a/libglusterfs/src/logging.c +++ b/libglusterfs/src/logging.c @@ -2302,6 +2302,8 @@ _gf_log_eh (const char *function, const char *fmt, ...) strcat (msg, str2); ret = eh_save_history (this->history, msg); + if (ret < 0) + GF_FREE (msg); out: GF_FREE (str1); diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index ca081854ba4..ff3f7c6eb24 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -111,6 +111,28 @@ out: return fd_ctx; } +static void +fuse_fd_ctx_destroy (xlator_t *this, fd_t *fd) +{ + fd_t *activefd = NULL; + uint64_t val = 0; + int ret = 0; + fuse_fd_ctx_t *fdctx = NULL; + + ret = fd_ctx_del (fd, this, &val); + if (!ret) { + fdctx = (fuse_fd_ctx_t *)(unsigned long)val; + if (fdctx) { + activefd = fdctx->activefd; + if (activefd) { + fd_unref (activefd); + } + + GF_FREE (fdctx); + } + } +} + fuse_fd_ctx_t * fuse_fd_ctx_get (xlator_t *this, fd_t *fd) { @@ -2560,10 +2582,12 @@ fuse_flush (xlator_t *this, fuse_in_header_t *finh, void *msg) int fuse_internal_release (xlator_t *this, fd_t *fd) { - //This is a place holder function to prevent "xlator does not implement - //release_cbk" Warning log. - //Actual release happens as part of fuse_release which gets executed - //when kernel fuse sends it. + /* This is important we cleanup our context here to avoid a leak + in case an error occurs and we get cleanup up by + call_unwind_error->...->args_wipe instead of the normal path. + */ + fuse_fd_ctx_destroy(this, fd); + return 0; } @@ -2571,12 +2595,8 @@ static void fuse_release (xlator_t *this, fuse_in_header_t *finh, void *msg) { struct fuse_release_in *fri = msg; - fd_t *activefd = NULL; fd_t *fd = NULL; - uint64_t val = 0; - int ret = 0; fuse_state_t *state = NULL; - fuse_fd_ctx_t *fdctx = NULL; fuse_private_t *priv = NULL; GET_STATE (this, finh, state); @@ -2591,18 +2611,7 @@ fuse_release (xlator_t *this, fuse_in_header_t *finh, void *msg) gf_log ("glusterfs-fuse", GF_LOG_TRACE, "%"PRIu64": RELEASE %p", finh->unique, state->fd); - ret = fd_ctx_del (fd, this, &val); - if (!ret) { - fdctx = (fuse_fd_ctx_t *)(unsigned long)val; - if (fdctx) { - activefd = fdctx->activefd; - if (activefd) { - fd_unref (activefd); - } - - GF_FREE (fdctx); - } - } + fuse_fd_ctx_destroy(this, state->fd); fd_unref (fd); state->fd = NULL; @@ -3060,11 +3069,7 @@ static void fuse_releasedir (xlator_t *this, fuse_in_header_t *finh, void *msg) { struct fuse_release_in *fri = msg; - fd_t *activefd = NULL; - uint64_t val = 0; - int ret = 0; fuse_state_t *state = NULL; - fuse_fd_ctx_t *fdctx = NULL; fuse_private_t *priv = NULL; GET_STATE (this, finh, state); @@ -3079,20 +3084,7 @@ fuse_releasedir (xlator_t *this, fuse_in_header_t *finh, void *msg) gf_log ("glusterfs-fuse", GF_LOG_TRACE, "%"PRIu64": RELEASEDIR %p", finh->unique, state->fd); - ret = fd_ctx_del (state->fd, this, &val); - - if (!ret) { - fdctx = (fuse_fd_ctx_t *)(unsigned long)val; - if (fdctx) { - activefd = fdctx->activefd; - if (activefd) { - fd_unref (activefd); - } - - GF_FREE (fdctx); - } - } - + fuse_fd_ctx_destroy(this, state->fd); fd_unref (state->fd); gf_fdptr_put (priv->fdtable, state->fd); -- cgit