From 75140d6150264ba50a47e104c3ce1ae40bd3f0c8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 18 Mar 2006 10:38:38 +0000 Subject: r14540: fix a talloc hierachie problem, make sure file and search handles are cleaned up before anything else in the pvfs_state struct, as there destructors reply on a valid pvfs_state struct metze (This used to be commit aaa5d377b9b6145a83c0e686c7fbb7b561ae8988) --- source4/ntvfs/posix/pvfs_flush.c | 2 +- source4/ntvfs/posix/pvfs_open.c | 62 +++++++++++++++++++-------------------- source4/ntvfs/posix/pvfs_search.c | 39 +++++++++--------------- source4/ntvfs/posix/vfs_posix.c | 34 +++++++++++++++++---- source4/ntvfs/posix/vfs_posix.h | 44 ++++++++++++++++++++------- 5 files changed, 108 insertions(+), 73 deletions(-) diff --git a/source4/ntvfs/posix/pvfs_flush.c b/source4/ntvfs/posix/pvfs_flush.c index ad87ed623b..d21f257201 100644 --- a/source4/ntvfs/posix/pvfs_flush.c +++ b/source4/ntvfs/posix/pvfs_flush.c @@ -60,7 +60,7 @@ NTSTATUS pvfs_flush(struct ntvfs_module_context *ntvfs, } /* they are asking to flush all open files */ - for (f=pvfs->open_files;f;f=f->next) { + for (f=pvfs->files.list;f;f=f->next) { pvfs_flush_file(pvfs, f); } diff --git a/source4/ntvfs/posix/pvfs_open.c b/source4/ntvfs/posix/pvfs_open.c index 3160519f73..2724339323 100644 --- a/source4/ntvfs/posix/pvfs_open.c +++ b/source4/ntvfs/posix/pvfs_open.c @@ -43,7 +43,7 @@ struct pvfs_file *pvfs_find_fd(struct pvfs_state *pvfs, { struct pvfs_file *f; - f = idr_find(pvfs->idtree_fnum, fnum); + f = idr_find(pvfs->files.idtree, fnum); if (f == NULL) { return NULL; } @@ -114,8 +114,8 @@ static int pvfs_dir_handle_destructor(void *p) static int pvfs_dir_fnum_destructor(void *p) { struct pvfs_file *f = p; - DLIST_REMOVE(f->pvfs->open_files, f); - idr_remove(f->pvfs->idtree_fnum, f->fnum); + DLIST_REMOVE(f->pvfs->files.list, f); + idr_remove(f->pvfs->files.idtree, f->fnum); return 0; } @@ -246,7 +246,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, return NT_STATUS_NO_MEMORY; } - fnum = idr_get_new_above(pvfs->idtree_fnum, f, PVFS_MIN_DIR_FNUM, UINT16_MAX); + fnum = idr_get_new_above(pvfs->files.idtree, f, PVFS_MIN_DIR_FNUM, UINT16_MAX); if (fnum == -1) { return NT_STATUS_TOO_MANY_OPENED_FILES; } @@ -258,7 +258,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, status = pvfs_access_check_create(pvfs, req, name, &access_mask); } if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return status; } @@ -295,7 +295,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, /* form the lock context used for opendb locking */ status = pvfs_locking_key(name, f->handle, &f->handle->odb_locking_key); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, f->fnum); + idr_remove(pvfs->files.idtree, f->fnum); return status; } @@ -306,7 +306,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, name->full_name)); /* we were supposed to do a blocking lock, so something is badly wrong! */ - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return NT_STATUS_INTERNAL_DB_CORRUPTION; } @@ -315,7 +315,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, share_access, access_mask, del_on_close, name->full_name); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, f->fnum); + idr_remove(pvfs->files.idtree, f->fnum); talloc_free(lck); return status; } @@ -323,7 +323,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, f->handle->have_opendb_entry = True; } - DLIST_ADD(pvfs->open_files, f); + DLIST_ADD(pvfs->files.list, f); /* setup destructors to avoid leaks on abnormal termination */ talloc_set_destructor(f->handle, pvfs_dir_handle_destructor); @@ -334,7 +334,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, mode_t mode = pvfs_fileperms(pvfs, attrib); if (mkdir(name->full_name, mode) == -1) { - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return pvfs_map_errno(pvfs,errno); } @@ -353,7 +353,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, /* form the lock context used for opendb locking */ status = pvfs_locking_key(name, f->handle, &f->handle->odb_locking_key); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, f->fnum); + idr_remove(pvfs->files.idtree, f->fnum); return status; } @@ -363,7 +363,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, name->full_name)); /* we were supposed to do a blocking lock, so something is badly wrong! */ - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return NT_STATUS_INTERNAL_DB_CORRUPTION; } @@ -382,7 +382,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, } if (!name->exists) { - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return NT_STATUS_OBJECT_NAME_NOT_FOUND; } @@ -406,7 +406,7 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs, return NT_STATUS_OK; cleanup_delete: - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); rmdir(name->full_name); return status; } @@ -493,9 +493,9 @@ static int pvfs_fnum_destructor(void *p) { struct pvfs_file *f = p; - DLIST_REMOVE(f->pvfs->open_files, f); + DLIST_REMOVE(f->pvfs->files.list, f); pvfs_lock_close(f->pvfs, f); - idr_remove(f->pvfs->idtree_fnum, f->fnum); + idr_remove(f->pvfs->files.idtree, f->fnum); return 0; } @@ -591,7 +591,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, return NT_STATUS_NO_MEMORY; } - fnum = idr_get_new_above(pvfs->idtree_fnum, f, PVFS_MIN_NEW_FNUM, UINT16_MAX); + fnum = idr_get_new_above(pvfs->files.idtree, f, PVFS_MIN_NEW_FNUM, UINT16_MAX); if (fnum == -1) { return NT_STATUS_TOO_MANY_OPENED_FILES; } @@ -602,7 +602,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, /* create the file */ fd = open(name->full_name, flags | O_CREAT | O_EXCL, mode); if (fd == -1) { - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return pvfs_map_errno(pvfs, errno); } @@ -612,7 +612,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, if (name->stream_name) { status = pvfs_stream_create(pvfs, name, fd); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); close(fd); return status; } @@ -621,7 +621,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, /* re-resolve the open fd */ status = pvfs_resolve_name_fd(pvfs, fd, name); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); close(fd); return status; } @@ -674,7 +674,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, /* bad news, we must have hit a race - we don't delete the file here as the most likely scenario is that someone else created the file at the same time */ - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); close(fd); return status; } @@ -699,7 +699,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, f->handle->have_opendb_entry = True; f->handle->sticky_write_time = False; - DLIST_ADD(pvfs->open_files, f); + DLIST_ADD(pvfs->files.list, f); /* setup a destructor to avoid file descriptor leaks on abnormal termination */ @@ -731,7 +731,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs, return NT_STATUS_OK; cleanup_delete: - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); close(fd); unlink(name->full_name); return status; @@ -845,7 +845,7 @@ static NTSTATUS pvfs_open_deny_dos(struct ntvfs_module_context *ntvfs, circumstances you actually get the _same_ handle back twice, rather than a new handle. */ - for (f2=pvfs->open_files;f2;f2=f2->next) { + for (f2=pvfs->files.list;f2;f2=f2->next) { if (f2 != f && f2->session_info == req->session_info && f2->smbpid == req->smbpid && @@ -1099,7 +1099,7 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs, } /* allocate a fnum */ - fnum = idr_get_new_above(pvfs->idtree_fnum, f, PVFS_MIN_FILE_FNUM, UINT16_MAX); + fnum = idr_get_new_above(pvfs->files.idtree, f, PVFS_MIN_FILE_FNUM, UINT16_MAX); if (fnum == -1) { return NT_STATUS_TOO_MANY_OPENED_FILES; } @@ -1128,13 +1128,13 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs, opendb locking */ status = pvfs_locking_key(name, f->handle, &f->handle->odb_locking_key); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, f->fnum); + idr_remove(pvfs->files.idtree, f->fnum); return status; } status = pvfs_brl_locking_key(name, f->handle, &f->handle->brl_locking_key); if (!NT_STATUS_IS_OK(status)) { - idr_remove(pvfs->idtree_fnum, f->fnum); + idr_remove(pvfs->files.idtree, f->fnum); return status; } @@ -1145,11 +1145,11 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs, name->full_name)); /* we were supposed to do a blocking lock, so something is badly wrong! */ - idr_remove(pvfs->idtree_fnum, fnum); + idr_remove(pvfs->files.idtree, fnum); return NT_STATUS_INTERNAL_DB_CORRUPTION; } - DLIST_ADD(pvfs->open_files, f); + DLIST_ADD(pvfs->files.list, f); /* setup a destructor to avoid file descriptor leaks on abnormal termination */ @@ -1308,7 +1308,7 @@ NTSTATUS pvfs_logoff(struct ntvfs_module_context *ntvfs, struct pvfs_state *pvfs = ntvfs->private_data; struct pvfs_file *f, *next; - for (f=pvfs->open_files;f;f=next) { + for (f=pvfs->files.list;f;f=next) { next = f->next; if (f->session_info == req->session_info) { talloc_free(f); @@ -1328,7 +1328,7 @@ NTSTATUS pvfs_exit(struct ntvfs_module_context *ntvfs, struct pvfs_state *pvfs = ntvfs->private_data; struct pvfs_file *f, *next; - for (f=pvfs->open_files;f;f=next) { + for (f=pvfs->files.list;f;f=next) { next = f->next; if (f->session_info == req->session_info && f->smbpid == req->smbpid) { diff --git a/source4/ntvfs/posix/pvfs_search.c b/source4/ntvfs/posix/pvfs_search.c index 7ee64887fb..32bef1ae53 100644 --- a/source4/ntvfs/posix/pvfs_search.c +++ b/source4/ntvfs/posix/pvfs_search.c @@ -26,22 +26,7 @@ #include "librpc/gen_ndr/security.h" #include "smbd/service_stream.h" #include "lib/events/events.h" - - -/* the state of a search started with pvfs_search_first() */ -struct pvfs_search_state { - struct pvfs_state *pvfs; - uint16_t handle; - uint_t current_index; - uint16_t search_attrib; - uint16_t must_attrib; - struct pvfs_dir *dir; - time_t last_used; - uint_t num_ea_names; - struct ea_name *ea_names; - struct timed_event *te; -}; - +#include "dlinklist.h" /* place a reasonable limit on old-style searches as clients tend to not send search close requests */ @@ -53,7 +38,8 @@ struct pvfs_search_state { static int pvfs_search_destructor(void *ptr) { struct pvfs_search_state *search = ptr; - idr_remove(search->pvfs->idtree_search, search->handle); + DLIST_REMOVE(search->pvfs->search.list, search); + idr_remove(search->pvfs->search.idtree, search->handle); return 0; } @@ -75,7 +61,7 @@ static void pvfs_search_setup_timer(struct pvfs_search_state *search) struct event_context *ev = search->pvfs->ntvfs->ctx->event_ctx; talloc_free(search->te); search->te = event_add_timed(ev, search, - timeval_current_ofs(search->pvfs->search_inactivity_time, 0), + timeval_current_ofs(search->pvfs->search.inactivity_time, 0), pvfs_search_timer, search); } @@ -306,7 +292,7 @@ static void pvfs_search_cleanup(struct pvfs_state *pvfs) time_t t = time(NULL); for (i=0;iidtree_search, i); + struct pvfs_search_state *search = idr_find(pvfs->search.idtree, i); if (search == NULL) return; if (pvfs_list_eos(search->dir, search->current_index) && search->last_used != 0 && @@ -371,10 +357,10 @@ static NTSTATUS pvfs_search_first_old(struct ntvfs_module_context *ntvfs, /* we need to give a handle back to the client so it can continue a search */ - id = idr_get_new(pvfs->idtree_search, search, MAX_OLD_SEARCHES); + id = idr_get_new(pvfs->search.idtree, search, MAX_OLD_SEARCHES); if (id == -1) { pvfs_search_cleanup(pvfs); - id = idr_get_new(pvfs->idtree_search, search, MAX_OLD_SEARCHES); + id = idr_get_new(pvfs->search.idtree, search, MAX_OLD_SEARCHES); } if (id == -1) { return NT_STATUS_INSUFFICIENT_RESOURCES; @@ -389,6 +375,8 @@ static NTSTATUS pvfs_search_first_old(struct ntvfs_module_context *ntvfs, search->last_used = time(NULL); search->te = NULL; + DLIST_ADD(pvfs->search.list, search); + talloc_set_destructor(search, pvfs_search_destructor); status = pvfs_search_fill(pvfs, req, io->search_first.in.max_count, search, io->generic.level, @@ -425,7 +413,7 @@ static NTSTATUS pvfs_search_next_old(struct ntvfs_module_context *ntvfs, handle = io->search_next.in.id.handle | (io->search_next.in.id.reserved<<8); max_count = io->search_next.in.max_count; - search = idr_find(pvfs->idtree_search, handle); + search = idr_find(pvfs->search.idtree, handle); if (search == NULL) { /* we didn't find the search handle */ return NT_STATUS_INVALID_HANDLE; @@ -506,7 +494,7 @@ NTSTATUS pvfs_search_first(struct ntvfs_module_context *ntvfs, return status; } - id = idr_get_new(pvfs->idtree_search, search, UINT16_MAX); + id = idr_get_new(pvfs->search.idtree, search, UINT16_MAX); if (id == -1) { return NT_STATUS_INSUFFICIENT_RESOURCES; } @@ -522,6 +510,7 @@ NTSTATUS pvfs_search_first(struct ntvfs_module_context *ntvfs, search->ea_names = io->t2ffirst.in.ea_names; search->te = NULL; + DLIST_ADD(pvfs->search.list, search); talloc_set_destructor(search, pvfs_search_destructor); status = pvfs_search_fill(pvfs, req, max_count, search, io->generic.level, @@ -571,7 +560,7 @@ NTSTATUS pvfs_search_next(struct ntvfs_module_context *ntvfs, handle = io->t2fnext.in.handle; - search = idr_find(pvfs->idtree_search, handle); + search = idr_find(pvfs->search.idtree, handle); if (search == NULL) { /* we didn't find the search handle */ return NT_STATUS_INVALID_HANDLE; @@ -631,7 +620,7 @@ NTSTATUS pvfs_search_close(struct ntvfs_module_context *ntvfs, handle = io->findclose.in.handle; } - search = idr_find(pvfs->idtree_search, handle); + search = idr_find(pvfs->search.idtree, handle); if (search == NULL) { /* we didn't find the search handle */ return NT_STATUS_INVALID_HANDLE; diff --git a/source4/ntvfs/posix/vfs_posix.c b/source4/ntvfs/posix/vfs_posix.c index 3f1c676df3..cb441cb4c9 100644 --- a/source4/ntvfs/posix/vfs_posix.c +++ b/source4/ntvfs/posix/vfs_posix.c @@ -57,7 +57,7 @@ static void pvfs_setup_options(struct pvfs_state *pvfs) pvfs->alloc_size_rounding = lp_parm_int(snum, "posix", "allocationrounding", 512); - pvfs->search_inactivity_time = lp_parm_int(snum, + pvfs->search.inactivity_time = lp_parm_int(snum, "posix", "searchinactivity", 300); #if HAVE_XATTR_SUPPORT @@ -104,6 +104,28 @@ static void pvfs_setup_options(struct pvfs_state *pvfs) } } +static int pvfs_state_destructor(void *ptr) +{ + struct pvfs_state *pvfs = talloc_get_type(ptr, struct pvfs_state); + struct pvfs_file *f, *fn; + struct pvfs_search_state *s, *sn; + + /* + * make sure we cleanup files and searches before anythingelse + * because there destructors need to acess the pvfs_state struct + */ + for (f=pvfs->files.list; f; f=fn) { + fn = f->next; + talloc_free(f); + } + + for (s=pvfs->search.list; s; s=sn) { + sn = s->next; + talloc_free(s); + } + + return 0; +} /* connect to a share - used when a tree_connect operation comes @@ -169,18 +191,20 @@ static NTSTATUS pvfs_connect(struct ntvfs_module_context *ntvfs, } /* allocate the fnum id -> ptr tree */ - pvfs->idtree_fnum = idr_init(pvfs); - NT_STATUS_HAVE_NO_MEMORY(pvfs->idtree_fnum); + pvfs->files.idtree = idr_init(pvfs); + NT_STATUS_HAVE_NO_MEMORY(pvfs->files.idtree); /* allocate the search handle -> ptr tree */ - pvfs->idtree_search = idr_init(pvfs); - NT_STATUS_HAVE_NO_MEMORY(pvfs->idtree_search); + pvfs->search.idtree = idr_init(pvfs); + NT_STATUS_HAVE_NO_MEMORY(pvfs->search.idtree); status = pvfs_mangle_init(pvfs); NT_STATUS_NOT_OK_RETURN(status); pvfs_setup_options(pvfs); + talloc_set_destructor(pvfs, pvfs_state_destructor); + #ifdef SIGXFSZ /* who had the stupid idea to generate a signal on a large file write instead of just failing it!? */ diff --git a/source4/ntvfs/posix/vfs_posix.h b/source4/ntvfs/posix/vfs_posix.h index f75e7d3563..59e4ec1abf 100644 --- a/source4/ntvfs/posix/vfs_posix.h +++ b/source4/ntvfs/posix/vfs_posix.h @@ -38,20 +38,12 @@ struct pvfs_state { const char *share_name; uint_t flags; - struct pvfs_file *open_files; - struct pvfs_mangle_context *mangle_ctx; struct brl_context *brl_context; struct odb_context *odb_context; struct sidmap_context *sidmap; - /* an id tree mapping open search ID to a pvfs_search_state structure */ - struct idr_context *idtree_search; - - /* an id tree mapping open file handle -> struct pvfs_file */ - struct idr_context *idtree_fnum; - /* a list of pending async requests. Needed to support ntcancel */ struct pvfs_wait *wait_list; @@ -68,9 +60,25 @@ struct pvfs_state { /* the allocation size rounding */ uint32_t alloc_size_rounding; - /* how long to keep inactive searches around for */ - uint_t search_inactivity_time; - + struct { + /* an id tree mapping open file handle -> struct pvfs_file */ + struct idr_context *idtree; + + /* the open files as DLINKLIST */ + struct pvfs_file *list; + } files; + + struct { + /* an id tree mapping open search ID to a pvfs_search_state structure */ + struct idr_context *idtree; + + /* the open searches as DLINKLIST */ + struct pvfs_search_state *list; + + /* how long to keep inactive searches around for */ + uint_t inactivity_time; + } search; + /* used to accelerate acl mapping */ struct { const struct dom_sid *creator_owner; @@ -173,6 +181,20 @@ struct pvfs_file { uint64_t lock_count; }; +/* the state of a search started with pvfs_search_first() */ +struct pvfs_search_state { + struct pvfs_search_state *prev, *next; + struct pvfs_state *pvfs; + uint16_t handle; + uint_t current_index; + uint16_t search_attrib; + uint16_t must_attrib; + struct pvfs_dir *dir; + time_t last_used; + uint_t num_ea_names; + struct ea_name *ea_names; + struct timed_event *te; +}; /* flags to pvfs_resolve_name() */ #define PVFS_RESOLVE_WILDCARD (1<<0) -- cgit