From 99c2693c620cd222da5561d526aa328bec426b77 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 15 Dec 2000 21:29:06 +0000 Subject: Added lock backouts on fail. When chaining together long lines of bloody "if" statements, which should logically be separated, and one of them allocates memory, remember to *free* it *WHETHER OR NOT THE IF STATEMENTS SUCCEEDED* !!!! Yes I do consider this a bug in the coding style of Tridge, Rusty, Tim et al. :-). I'm just pissed 'cos this took 4 hours to track down even with an insure error report stating me in the face and also Ben Woodward looking over the code with me :-). Jeremy. (This used to be commit 506b5e34c3ba16768dbc82ba21044787de160c45) --- source3/include/proto.h | 3 --- source3/rpc_client/cli_srvsvc.c | 2 -- source3/rpc_parse/parse_srv.c | 41 ++--------------------------------------- source3/rpc_server/srv_srvsvc.c | 31 +++++++++++++------------------ source3/tdb/tdb.c | 31 ++++++++++++++++++++++++++++--- 5 files changed, 43 insertions(+), 65 deletions(-) diff --git a/source3/include/proto.h b/source3/include/proto.h index 79c742e0a8..2112187865 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -3059,9 +3059,6 @@ void init_srv_share_info2(SH_INFO_2 *sh2, char *net_name, uint32 type, char *remark, uint32 perms, uint32 max_uses, uint32 num_uses, char *path, char *passwd); -void free_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr); -void free_srv_q_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n); -void free_srv_r_net_share_enum(SRV_R_NET_SHARE_ENUM *r_n); void init_srv_q_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n, char *srv_name, uint32 info_level, uint32 preferred_len, ENUM_HND *hnd); diff --git a/source3/rpc_client/cli_srvsvc.c b/source3/rpc_client/cli_srvsvc.c index b6dc0aff4a..4a3aed6d15 100644 --- a/source3/rpc_client/cli_srvsvc.c +++ b/source3/rpc_client/cli_srvsvc.c @@ -243,7 +243,6 @@ BOOL do_srv_net_srv_share_enum(struct cli_state *cli, /* report error code */ DEBUG(0,("SRV_R_NET_SHARE_ENUM: %s\n", get_nt_error_msg(r_o->status))); prs_mem_free(&rdata); - free_srv_r_net_share_enum(r_o); return False; } @@ -252,7 +251,6 @@ BOOL do_srv_net_srv_share_enum(struct cli_state *cli, DEBUG(0,("SRV_R_NET_SHARE_ENUM: info class %d does not match request %d\n", r_o->ctr.switch_value, switch_value)); prs_mem_free(&rdata); - free_srv_r_net_share_enum(r_o); return False; } diff --git a/source3/rpc_parse/parse_srv.c b/source3/rpc_parse/parse_srv.c index be7401ffc5..69cc1e7bda 100644 --- a/source3/rpc_parse/parse_srv.c +++ b/source3/rpc_parse/parse_srv.c @@ -291,7 +291,7 @@ static BOOL srv_io_srv_share_ctr(char *desc, SRV_SHARE_INFO_CTR *ctr, prs_struct int i; if (UNMARSHALLING(ps)) { - if (!(info1 = malloc(num_entries * sizeof(SRV_SHARE_INFO_1)))) + if (!(info1 = prs_alloc_mem(ps, num_entries * sizeof(SRV_SHARE_INFO_1)))) return False; memset(info1, '\0', num_entries * sizeof(SRV_SHARE_INFO_1)); ctr->share.info1 = info1; @@ -317,7 +317,7 @@ static BOOL srv_io_srv_share_ctr(char *desc, SRV_SHARE_INFO_CTR *ctr, prs_struct int i; if (UNMARSHALLING(ps)) { - if (!(info2 = malloc(num_entries * sizeof(SRV_SHARE_INFO_2)))) + if (!(info2 = prs_alloc_mem(ps,num_entries * sizeof(SRV_SHARE_INFO_2)))) return False; memset(info2, '\0', num_entries * sizeof(SRV_SHARE_INFO_2)); ctr->share.info2 = info2; @@ -345,43 +345,6 @@ static BOOL srv_io_srv_share_ctr(char *desc, SRV_SHARE_INFO_CTR *ctr, prs_struct return True; } -/******************************************************************* - Frees a SRV_SHARE_INFO_CTR structure. -********************************************************************/ - -void free_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr) -{ - if(!ctr) - return; - if(ctr->share.info) - free(ctr->share.info); - memset(ctr, '\0', sizeof(SRV_SHARE_INFO_CTR)); -} - -/******************************************************************* - Frees a SRV_Q_NET_SHARE_ENUM structure. -********************************************************************/ - -void free_srv_q_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n) -{ - if(!q_n) - return; - free_srv_share_info_ctr(&q_n->ctr); - memset(q_n, '\0', sizeof(SRV_Q_NET_SHARE_ENUM)); -} - -/******************************************************************* - Frees a SRV_R_NET_SHARE_ENUM structure. -********************************************************************/ - -void free_srv_r_net_share_enum(SRV_R_NET_SHARE_ENUM *r_n) -{ - if(!r_n) - return; - free_srv_share_info_ctr(&r_n->ctr); - memset(r_n, '\0', sizeof(SRV_R_NET_SHARE_ENUM)); -} - /******************************************************************* Inits a SRV_Q_NET_SHARE_ENUM structure. ********************************************************************/ diff --git a/source3/rpc_server/srv_srvsvc.c b/source3/rpc_server/srv_srvsvc.c index 715a681a29..52ae54fd94 100644 --- a/source3/rpc_server/srv_srvsvc.c +++ b/source3/rpc_server/srv_srvsvc.c @@ -109,7 +109,7 @@ static void init_srv_share_info_1005(SRV_SHARE_INFO_1005* sh1005, int snum) Fill in a share info structure. ********************************************************************/ -static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr, +static BOOL init_srv_share_info_ctr(TALLOC_CTX *ctx, SRV_SHARE_INFO_CTR *ctr, uint32 info_level, uint32 *resume_hnd, uint32 *total_entries) { int num_entries = 0; @@ -142,7 +142,7 @@ static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr, SRV_SHARE_INFO_1 *info1; int i = 0; - info1 = malloc(num_entries * sizeof(SRV_SHARE_INFO_1)); + info1 = talloc(ctx, num_entries * sizeof(SRV_SHARE_INFO_1)); for (snum = *resume_hnd; snum < num_services; snum++) { if (lp_browseable(snum) && lp_snum_ok(snum)) { @@ -159,7 +159,7 @@ static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr, SRV_SHARE_INFO_2 *info2; int i = 0; - info2 = malloc(num_entries * sizeof(SRV_SHARE_INFO_2)); + info2 = talloc(ctx, num_entries * sizeof(SRV_SHARE_INFO_2)); for (snum = *resume_hnd; snum < num_services; snum++) { if (lp_browseable(snum) && lp_snum_ok(snum)) { @@ -183,12 +183,12 @@ static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr, Inits a SRV_R_NET_SHARE_ENUM structure. ********************************************************************/ -static void init_srv_r_net_share_enum(SRV_R_NET_SHARE_ENUM *r_n, +static void init_srv_r_net_share_enum(TALLOC_CTX *ctx, SRV_R_NET_SHARE_ENUM *r_n, uint32 info_level, uint32 resume_hnd) { DEBUG(5,("init_srv_r_net_share_enum: %d\n", __LINE__)); - if (init_srv_share_info_ctr(&r_n->ctr, info_level, + if (init_srv_share_info_ctr(ctx, &r_n->ctr, info_level, &resume_hnd, &r_n->total_entries)) { r_n->status = 0x0; } else { @@ -207,21 +207,25 @@ static BOOL srv_reply_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n, { SRV_R_NET_SHARE_ENUM r_n; BOOL ret; + TALLOC_CTX *ctx = talloc_init(); DEBUG(5,("srv_net_share_enum: %d\n", __LINE__)); + if (!ctx) { + DEBUG(0,("srv_reply_net_share_enum: talloc_init failed.\n")); + return False; + } + /* Create the list of shares for the response. */ - init_srv_r_net_share_enum(&r_n, + init_srv_r_net_share_enum(ctx, &r_n, q_n->ctr.info_level, get_enum_hnd(&q_n->enum_hnd)); /* store the response in the SMB stream */ ret = srv_io_r_net_share_enum("", &r_n, rdata, 0); - /* Free the memory used by the response. */ - free_srv_r_net_share_enum(&r_n); - DEBUG(5,("srv_net_share_enum: %d\n", __LINE__)); + talloc_destroy(ctx); return ret; } @@ -286,9 +290,6 @@ static BOOL srv_reply_net_share_get_info(SRV_Q_NET_SHARE_GET_INFO *q_n, /* store the response in the SMB stream */ ret = srv_io_r_net_share_get_info("", &r_n, rdata, 0); - /* Free the memory used by the response. */ - free_srv_r_net_share_get_info(&r_n); - DEBUG(5,("srv_net_share_get_info: %d\n", __LINE__)); return ret; @@ -1024,9 +1025,6 @@ static BOOL api_srv_net_share_enum(pipes_struct *p) ret = srv_reply_net_share_enum(&q_n, rdata); - /* Free any data allocated in the unmarshalling. */ - free_srv_q_net_share_enum(&q_n); - return ret; } @@ -1049,9 +1047,6 @@ static BOOL api_srv_net_share_get_info(pipes_struct *p) ret = srv_reply_net_share_get_info(&q_n, rdata); - /* Free any data allocated in the unmarshalling. */ - free_srv_q_net_share_get_info(&q_n); - return ret; } diff --git a/source3/tdb/tdb.c b/source3/tdb/tdb.c index ede38f27f9..afc87b7da0 100644 --- a/source3/tdb/tdb.c +++ b/source3/tdb/tdb.c @@ -922,11 +922,13 @@ TDB_DATA tdb_nextkey(TDB_CONTEXT *tdb, TDB_DATA oldkey) rec.key_len)) || memcmp(k, oldkey.dptr, oldkey.dsize) != 0) { /* No, it wasn't: unlock it and start from scratch */ - free(k); unlock_record(tdb, tdb->travlocks.off); tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK); tdb->travlocks.off = 0; } + + if (k) + free(k); } if (!tdb->travlocks.off) { @@ -1180,7 +1182,19 @@ int tdb_lockall(TDB_CONTEXT *tdb) /* There are no locks on read-only dbs */ if (tdb->read_only) return TDB_ERRCODE(TDB_ERR_LOCK, -1); if (tdb->lockedkeys) return TDB_ERRCODE(TDB_ERR_NOLOCK, -1); - for (i = 0; i < tdb->header.hash_size; i++) tdb_lock(tdb, i, F_WRLCK); + for (i = 0; i < tdb->header.hash_size; i++) + if (tdb_lock(tdb, i, F_WRLCK)) + break; + + /* If error, release locks we have... */ + if (i < tdb->header.hash_size) { + u32 j; + + for ( j = 0; j < i; j++) + tdb_unlock(tdb, j, F_WRLCK); + return TDB_ERRCODE(TDB_ERR_NOLOCK, -1); + } + return 0; } void tdb_unlockall(TDB_CONTEXT *tdb) @@ -1211,7 +1225,18 @@ int tdb_lockkeys(TDB_CONTEXT *tdb, u32 number, TDB_DATA keys[]) tdb->lockedkeys[j+1] = hash; } /* Finally, lock in order */ - for (i = 0; i < number; i++) tdb_lock(tdb, i, F_WRLCK); + for (i = 0; i < number; i++) + if (tdb_lock(tdb, i, F_WRLCK)) + break; + + /* If error, release locks we have... */ + if (i < number) { + for ( j = 0; j < i; j++) + tdb_unlock(tdb, j, F_WRLCK); + free(tdb->lockedkeys); + tdb->lockedkeys = NULL; + return TDB_ERRCODE(TDB_ERR_NOLOCK, -1); + } return 0; } -- cgit