From 95cfcda13fe9a70b9955a7c44173d619eacb34c1 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 9 Mar 2020 11:54:37 +0100 Subject: [PATCH] vfs_default: Protect vfs_getxattrat_done() from accessing a freed req pointer If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the request is in flight (share forced closed by smbcontrol), then we set state->req = NULL in the state destructor. The existing state destructor prevents the state memory from being freed, so when the thread completes and calls vfs_getxattrat_done(), just throw away the result if state->req == NULL. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/modules/vfs_default.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index bd39eb47e9a..cf24e66e048 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3289,6 +3289,15 @@ struct vfswrap_getxattrat_state { static int vfswrap_getxattrat_state_destructor( struct vfswrap_getxattrat_state *state) { + /* + * This destructor only gets called if the request is still + * in flight, which is why we deny it by returning -1. We + * also set the req pointer to NULL so the _done function + * can detect the caller doesn't want the result anymore. + * + * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. + */ + state->req = NULL; return -1; } @@ -3519,6 +3528,16 @@ static void vfswrap_getxattrat_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); + if (req == NULL) { + /* + * We were shutdown closed in flight. No one wants the result, + * and state has been reparented to the NULL context, so just + * free it so we don't leak memory. + */ + DBG_NOTICE("getxattr request abandoned in flight\n"); + TALLOC_FREE(state); + return; + } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.47.2