diff options
author | Sverker Eriksson <[email protected]> | 2014-02-19 16:32:12 +0100 |
---|---|---|
committer | Sverker Eriksson <[email protected]> | 2014-02-19 16:32:12 +0100 |
commit | 8c17e706e212b859f1a248f43c8091e8d541b08e (patch) | |
tree | 39bb2de5f3d2c11cb9bbb20144e56486d710171d /erts | |
parent | 9fbb7a9a72a9b9f2bc12aebdbc89cac4bcd21873 (diff) | |
parent | 651b949d85f847318ece301b64c099ad7959e9d0 (diff) | |
download | otp-8c17e706e212b859f1a248f43c8091e8d541b08e.tar.gz otp-8c17e706e212b859f1a248f43c8091e8d541b08e.tar.bz2 otp-8c17e706e212b859f1a248f43c8091e8d541b08e.zip |
Merge branch 'sverk/crypto/hmac-context-reuse-bug'
OTP-11722
OTP-11724
* sverk/crypto/hmac-context-reuse-bug:
crypto: Fix bug when using old hmac context
erts: Fix NIF bug when load/upgrade fails after enif_open_resource_type
Conflicts:
erts/emulator/test/nif_SUITE.erl
Diffstat (limited to 'erts')
-rw-r--r-- | erts/emulator/beam/erl_nif.c | 111 | ||||
-rw-r--r-- | erts/emulator/test/nif_SUITE.erl | 134 | ||||
-rw-r--r-- | erts/emulator/test/nif_SUITE_data/nif_SUITE.c | 1 | ||||
-rw-r--r-- | erts/emulator/test/nif_SUITE_data/nif_mod.c | 47 |
4 files changed, 254 insertions, 39 deletions
diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c index c35f1fc2c6..4126df6f34 100644 --- a/erts/emulator/beam/erl_nif.c +++ b/erts/emulator/beam/erl_nif.c @@ -1237,6 +1237,19 @@ static void steal_resource_type(ErlNifResourceType* type) } } +/* The opened_rt_list is used by enif_open_resource_type() + * in order to rollback "creates" and "take-overs" in case the load fails. + */ +struct opened_resource_type +{ + struct opened_resource_type* next; + + ErlNifResourceFlags op; + ErlNifResourceType* type; + ErlNifResourceDtor* new_dtor; +}; +static struct opened_resource_type* opened_rt_list = NULL; + ErlNifResourceType* enif_open_resource_type(ErlNifEnv* env, const char* module_str, @@ -1258,22 +1271,21 @@ enif_open_resource_type(ErlNifEnv* env, if (type == NULL) { if (flags & ERL_NIF_RT_CREATE) { type = erts_alloc(ERTS_ALC_T_NIF, - sizeof(struct enif_resource_type_t)); - type->dtor = dtor; + sizeof(struct enif_resource_type_t)); type->module = module_am; type->name = name_am; erts_refc_init(&type->refc, 1); - type->owner = env->mod_nif; - type->prev = &resource_type_list; - type->next = resource_type_list.next; - type->next->prev = type; - type->prev->next = type; op = ERL_NIF_RT_CREATE; + #ifdef DEBUG + type->dtor = (void*)1; + type->owner = (void*)2; + type->prev = (void*)3; + type->next = (void*)4; + #endif } } else { - if (flags & ERL_NIF_RT_TAKEOVER) { - steal_resource_type(type); + if (flags & ERL_NIF_RT_TAKEOVER) { op = ERL_NIF_RT_TAKEOVER; } else { @@ -1281,12 +1293,13 @@ enif_open_resource_type(ErlNifEnv* env, } } if (type != NULL) { - type->owner = env->mod_nif; - type->dtor = dtor; - if (type->dtor != NULL) { - erts_refc_inc(&type->owner->rt_dtor_cnt, 1); - } - erts_refc_inc(&type->owner->rt_cnt, 1); + struct opened_resource_type* ort = erts_alloc(ERTS_ALC_T_TMP, + sizeof(struct opened_resource_type)); + ort->op = op; + ort->type = type; + ort->new_dtor = dtor; + ort->next = opened_rt_list; + opened_rt_list = ort; } if (tried != NULL) { *tried = op; @@ -1294,6 +1307,51 @@ enif_open_resource_type(ErlNifEnv* env, return type; } +static void commit_opened_resource_types(struct erl_module_nif* lib) +{ + while (opened_rt_list) { + struct opened_resource_type* ort = opened_rt_list; + + ErlNifResourceType* type = ort->type; + + if (ort->op == ERL_NIF_RT_CREATE) { + type->prev = &resource_type_list; + type->next = resource_type_list.next; + type->next->prev = type; + type->prev->next = type; + } + else { /* ERL_NIF_RT_TAKEOVER */ + steal_resource_type(type); + } + + type->owner = lib; + type->dtor = ort->new_dtor; + + if (type->dtor != NULL) { + erts_refc_inc(&lib->rt_dtor_cnt, 1); + } + erts_refc_inc(&lib->rt_cnt, 1); + + opened_rt_list = ort->next; + erts_free(ERTS_ALC_T_TMP, ort); + } +} + +static void rollback_opened_resource_types(void) +{ + while (opened_rt_list) { + struct opened_resource_type* ort = opened_rt_list; + + if (ort->op == ERL_NIF_RT_CREATE) { + erts_free(ERTS_ALC_T_NIF, ort->type); + } + + opened_rt_list = ort->next; + erts_free(ERTS_ALC_T_TMP, ort); + } +} + + static void nif_resource_dtor(Binary* bin) { ErlNifResource* resource = (ErlNifResource*) ERTS_MAGIC_BIN_DATA(bin); @@ -1319,6 +1377,8 @@ void* enif_alloc_resource(ErlNifResourceType* type, size_t size) { Binary* bin = erts_create_magic_binary(SIZEOF_ErlNifResource(size), &nif_resource_dtor); ErlNifResource* resource = ERTS_MAGIC_BIN_DATA(bin); + + ASSERT(type->owner && type->next && type->prev); /* not allowed in load/upgrade */ resource->type = type; erts_refc_inc(&bin->refc, 1); #ifdef DEBUG @@ -2040,9 +2100,15 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) lib->entry = entry; erts_refc_init(&lib->rt_cnt, 0); erts_refc_init(&lib->rt_dtor_cnt, 0); + ASSERT(opened_rt_list == NULL); lib->mod = mod; env.mod_nif = lib; - if (mod->curr.nif != NULL) { /* Reload */ + if (mod->curr.nif != NULL) { /*************** Reload ******************/ + /* + * Repeated load_nif calls from same Erlang module instance ("reload") + * is deprecated and was only ment as a development feature not to + * be used in production systems. (See warning below) + */ int k; lib->priv_data = mod->curr.nif->priv_data; @@ -2074,6 +2140,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) ret = load_nif_error(BIF_P, reload, "Library reload-call unsuccessful."); } else { + commit_opened_resource_types(lib); mod->curr.nif->entry = NULL; /* to prevent 'unload' callback */ erts_unload_nif(mod->curr.nif); reload_warning = 1; @@ -2081,7 +2148,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) } else { lib->priv_data = NULL; - if (mod->old.nif != NULL) { /* Upgrade */ + if (mod->old.nif != NULL) { /**************** Upgrade ***************/ void* prev_old_data = mod->old.nif->priv_data; if (entry->upgrade == NULL) { ret = load_nif_error(BIF_P, upgrade, "Upgrade not supported by this NIF library."); @@ -2094,17 +2161,18 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) mod->old.nif->priv_data = prev_old_data; ret = load_nif_error(BIF_P, upgrade, "Library upgrade-call unsuccessful."); } - /*else if (mod->old_nif->priv_data != prev_old_data) { - refresh_cached_nif_data(mod->old_code, mod->old_nif); - }*/ + else + commit_opened_resource_types(lib); } - else if (entry->load != NULL) { /* Initial load */ + else if (entry->load != NULL) { /********* Initial load ***********/ erts_pre_nif(&env, BIF_P, lib); veto = entry->load(&env, &lib->priv_data, BIF_ARG_2); erts_post_nif(&env); if (veto) { ret = load_nif_error(BIF_P, "load", "Library load-call unsuccessful."); } + else + commit_opened_resource_types(lib); } } if (ret == am_ok) { @@ -2133,6 +2201,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) } else { error: + rollback_opened_resource_types(); ASSERT(ret != am_ok); if (lib != NULL) { erts_free(ERTS_ALC_T_NIF, lib); diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl index bcc1f9e5af..a854d3f05b 100644 --- a/erts/emulator/test/nif_SUITE.erl +++ b/erts/emulator/test/nif_SUITE.erl @@ -850,6 +850,138 @@ resource_takeover(Config) when is_list(Config) -> ?line ok = forget_resource(AN4), ?line [] = nif_mod_call_history(), + + %% + %% Test rollback after failed upgrade of same lib-version + %% + + {A5,BinA5} = make_resource(2, Holder, "A5"), + {NA5,BinNA5} = make_resource(0, Holder, "NA5"), + {AN5,_BinAN5} = make_resource(1, Holder, "AN5"), + + {A6,BinA6} = make_resource(2, Holder, "A6"), + {NA6,BinNA6} = make_resource(0, Holder, "NA6"), + {AN6,_BinAN6} = make_resource(1, Holder, "AN6"), + + {module,nif_mod} = erlang:load_module(nif_mod,ModBin), + undefined = nif_mod:lib_version(), + {error,{upgrade,_}} = + nif_mod:load_nif_lib(Config, 1, + [{resource_type, 4, ?RT_TAKEOVER, "resource_type_A",resource_dtor_B, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",null, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_TAKEOVER, "resource_type_A_null",resource_dtor_A, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_CREATE, "Mr Pink", resource_dtor_A, + ?RT_CREATE}, + + {return, 1} % FAIL + ]), + + undefined = nif_mod:lib_version(), + [{upgrade,1,5,105}] = nif_mod_call_history(), + + %% Make sure dtor was not changed (from A to B) + ok = forget_resource(A5), + [{{resource_dtor_A_v1,BinA5},1,6,106}] = nif_mod_call_history(), + + %% Make sure dtor was not nullified (from A to null) + ok = forget_resource(NA5), + [{{resource_dtor_A_v1,BinNA5},1,7,107}] = nif_mod_call_history(), + + %% Make sure dtor was not added (from null to A) + ok = forget_resource(AN5), + [] = nif_mod_call_history(), + + %% + %% Test rollback after failed upgrade of other lib-version + %% + + {error,{upgrade,_}} = + nif_mod:load_nif_lib(Config, 2, + [{resource_type, 4, ?RT_TAKEOVER, "resource_type_A",resource_dtor_B, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",null, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_TAKEOVER, "resource_type_A_null",resource_dtor_A, + ?RT_TAKEOVER}, + {resource_type, null, ?RT_TAKEOVER, "Mr Pink", resource_dtor_A, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_CREATE, "Mr Pink", resource_dtor_A, + ?RT_CREATE}, + + {return, 1} % FAIL + ]), + + undefined = nif_mod:lib_version(), + [{upgrade,2,_,_}] = nif_mod_call_history(), + + %% Make sure dtor was not changed (from A to B) + ok = forget_resource(A6), + [{{resource_dtor_A_v1,BinA6},1,_,_}] = nif_mod_call_history(), + + %% Make sure dtor was not nullified (from A to null) + ok = forget_resource(NA6), + [{{resource_dtor_A_v1,BinNA6},1,_,_}] = nif_mod_call_history(), + + %% Make sure dtor was not added (from null to A) + ok = forget_resource(AN6), + [] = nif_mod_call_history(), + + %% + %% Test rolback after failed initial load + %% + false = code:purge(nif_mod), + [{unload,1,_,_}] = nif_mod_call_history(), + true = code:delete(nif_mod), + false = code:purge(nif_mod), + [] = nif_mod_call_history(), + + + {module,nif_mod} = erlang:load_module(nif_mod,ModBin), + undefined = nif_mod:lib_version(), + {error,{load,_}} = + nif_mod:load_nif_lib(Config, 1, + [{resource_type, null, ?RT_TAKEOVER, "resource_type_A",resource_dtor_A, + ?RT_TAKEOVER}, + {resource_type, 4, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",null, + ?RT_CREATE}, + {resource_type, 4, ?RT_CREATE, "resource_type_A_null",resource_dtor_A, + ?RT_CREATE}, + {resource_type, 4, ?RT_CREATE, "Mr Pink", resource_dtor_A, + ?RT_CREATE}, + + {return, 1} % FAIL + ]), + + undefined = nif_mod:lib_version(), + ok = nif_mod:load_nif_lib(Config, 1, + [{resource_type, null, ?RT_TAKEOVER, "resource_type_A",resource_dtor_A, + ?RT_TAKEOVER}, + {resource_type, 0, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A", + resource_dtor_A, ?RT_CREATE}, + + {resource_type, 1, ?RT_CREATE, "resource_type_A_null", null, + ?RT_CREATE}, + {resource_type, null, ?RT_TAKEOVER, "Mr Pink", resource_dtor_A, + ?RT_TAKEOVER}, + + {return, 0} % SUCCESS + ]), + + ?line hold_nif_mod_priv_data(nif_mod:get_priv_data_ptr()), + ?line [{load,1,1,101},{get_priv_data_ptr,1,2,102}] = nif_mod_call_history(), + + {NA7,BinNA7} = make_resource(0, Holder, "NA7"), + {AN7,BinAN7} = make_resource(1, Holder, "AN7"), + + ok = forget_resource(NA7), + [{{resource_dtor_A_v1,BinNA7},1,_,_}] = nif_mod_call_history(), + + ok = forget_resource(AN7), + [] = nif_mod_call_history(), + ?line true = lists:member(?MODULE, erlang:system_info(taints)), ?line true = lists:member(nif_mod, erlang:system_info(taints)), ?line verify_tmpmem(TmpMem), @@ -1406,7 +1538,7 @@ dirty_nif(Config) when is_list(Config) -> {skipped,"No dirty scheduler support"} end. -next_msg(Pid) -> +next_msg(_Pid) -> receive M -> M after 100 -> diff --git a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c index b550d1f16d..160f4843ad 100644 --- a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c +++ b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c @@ -1477,7 +1477,6 @@ static ERL_NIF_TERM consume_timeslice_nif(ErlNifEnv* env, int argc, const ERL_NI { int percent; char atom[10]; - int do_repeat; if (!enif_get_int(env, argv[0], &percent) || !enif_get_atom(env, argv[1], atom, sizeof(atom), ERL_NIF_LATIN1)) { diff --git a/erts/emulator/test/nif_SUITE_data/nif_mod.c b/erts/emulator/test/nif_SUITE_data/nif_mod.c index e32d10057c..aed8524635 100644 --- a/erts/emulator/test/nif_SUITE_data/nif_mod.c +++ b/erts/emulator/test/nif_SUITE_data/nif_mod.c @@ -1,7 +1,7 @@ /* * %CopyrightBegin% * - * Copyright Ericsson AB 2009-2010. All Rights Reserved. + * Copyright Ericsson AB 2009-2014. All Rights Reserved. * * The contents of this file are subject to the Erlang Public License, * Version 1.1, (the "License"); you may not use this file except in @@ -41,6 +41,7 @@ static ERL_NIF_TERM am_null; static ERL_NIF_TERM am_resource_type; static ERL_NIF_TERM am_resource_dtor_A; static ERL_NIF_TERM am_resource_dtor_B; +static ERL_NIF_TERM am_return; static NifModPrivData* priv_data(ErlNifEnv* env) { @@ -54,6 +55,7 @@ static void init(ErlNifEnv* env) am_resource_type = enif_make_atom(env, "resource_type"); am_resource_dtor_A = enif_make_atom(env, "resource_dtor_A"); am_resource_dtor_B = enif_make_atom(env, "resource_dtor_B"); + am_return = enif_make_atom(env, "return"); } static void add_call_with_arg(ErlNifEnv* env, NifModPrivData* data, const char* func_name, @@ -105,19 +107,15 @@ static void resource_dtor_B(ErlNifEnv* env, void* a) } /* {resource_type, Ix|null, ErlNifResourceFlags in, "TypeName", dtor(A|B|null), ErlNifResourceFlags out}*/ -static void open_resource_type(ErlNifEnv* env, ERL_NIF_TERM op_tpl) +static void open_resource_type(ErlNifEnv* env, const ERL_NIF_TERM* arr) { NifModPrivData* data = priv_data(env); - const ERL_NIF_TERM* arr; - int arity; char rt_name[30]; union { ErlNifResourceFlags e; int i; } flags, exp_res, got_res; unsigned ix; ErlNifResourceDtor* dtor; ErlNifResourceType* got_ptr; - CHECK(enif_get_tuple(env, op_tpl, &arity, &arr)); - CHECK(arity == 6); CHECK(enif_is_identical(arr[0], am_resource_type)); CHECK(enif_get_int(env, arr[2], &flags.i)); CHECK(enif_get_string(env, arr[3], rt_name, sizeof(rt_name), ERL_NIF_LATIN1) > 0); @@ -147,18 +145,32 @@ static void open_resource_type(ErlNifEnv* env, ERL_NIF_TERM op_tpl) CHECK(got_res.e == exp_res.e); } -static void do_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info) +static void do_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info, int* retvalp) { NifModPrivData* data = priv_data(env); ERL_NIF_TERM head, tail; unsigned ix; + for (ix=0; ix<RT_MAX; ix++) { data->rt_arr[ix] = NULL; } for (head = load_info; enif_get_list_cell(env, head, &head, &tail); head = tail) { - - open_resource_type(env, head); + const ERL_NIF_TERM* arr; + int arity; + + CHECK(enif_get_tuple(env, head, &arity, &arr)); + switch (arity) { + case 6: + open_resource_type(env, arr); + break; + case 2: + CHECK(arr[0] == am_return); + CHECK(enif_get_int(env, arr[1], retvalp)); + break; + default: + CHECK(0); + } } CHECK(enif_is_empty_list(env, head)); } @@ -166,6 +178,7 @@ static void do_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info) static int load(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info) { NifModPrivData* data; + int retval = 0; init(env); data = (NifModPrivData*) enif_alloc(sizeof(NifModPrivData)); @@ -177,38 +190,40 @@ static int load(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info) add_call(env, data, "load"); - do_load_info(env, load_info); + do_load_info(env, load_info, &retval); data->calls = 0; - return 0; + return retval; } static int reload(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info) { NifModPrivData* data = (NifModPrivData*) *priv; + int retval = 0; init(env); add_call(env, data, "reload"); - do_load_info(env, load_info); - return 0; + do_load_info(env, load_info, &retval); + return retval; } static int upgrade(ErlNifEnv* env, void** priv, void** old_priv_data, ERL_NIF_TERM load_info) { NifModPrivData* data = (NifModPrivData*) *old_priv_data; + int retval = 0; init(env); add_call(env, data, "upgrade"); data->ref_cnt++; *priv = *old_priv_data; - do_load_info(env, load_info); + do_load_info(env, load_info, &retval); - return 0; + return retval; } static void unload(ErlNifEnv* env, void* priv) { NifModPrivData* data = (NifModPrivData*) priv; - int is_last; + add_call(env, data, "unload"); NifModPrivData_release(data); } |