From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <5559A3E1.2040106@windriver.com> Date: Mon, 18 May 2015 16:33:37 +0800 From: wenzong fan MIME-Version: 1.0 To: Stephen Smalley , Subject: Re: [PATCH] mcstransd: fix reload issue References: <1431483069-16408-1-git-send-email-wenzong.fan@windriver.com> <5554D0CC.4060208@tycho.nsa.gov> In-Reply-To: <5554D0CC.4060208@tycho.nsa.gov> Content-Type: text/plain; charset="windows-1252"; format=flowed List-Id: "Security-Enhanced Linux \(SELinux\) mailing list" List-Post: List-Help: On 05/15/2015 12:43 AM, Stephen Smalley wrote: > On 05/12/2015 10:11 PM, wenzong.fan@windriver.com wrote: >> From: Wenzong Fan >> >> Generally mcstransd works well on MLS enabled system, but if "Reload >> Translations" triggered, it will fail to translate any MLS level. >> >> * Why it fails: >> >> Check process_trans() in mcstrans.c: >> >> 723 process_trans(char *buffer) { >> 724 static domain_t *domain; >> [snip] ... >> 784 if (!domain) { >> 785 domain = create_domain("Default"); >> >> While reloading translations, the struct of domain will be destroyed >> but the static pointer will be kept, it becomes wild and prevents the >> create_domain() from running; Then invalid domain will be used for >> initializing hashtable that stores translations data. >> >> * Fix to it: >> >> Define local *domain to get the struct of domain always be initialized; >> Use static hashtable to store all translations data. >> >> Signed-off-by: Wenzong Fan >> --- >> policycoreutils/mcstrans/src/mcstrans.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/policycoreutils/mcstrans/src/mcstrans.c b/policycoreutils/mcstrans/src/mcstrans.c >> index 4d31857..e8eda32 100644 >> --- a/policycoreutils/mcstrans/src/mcstrans.c >> +++ b/policycoreutils/mcstrans/src/mcstrans.c >> @@ -100,11 +100,14 @@ typedef struct base_classification { >> struct base_classification *next; >> } base_classification_t; >> >> +static context_map_node_t *table_raw_to_trans[N_BUCKETS]; >> +static context_map_node_t *table_trans_to_raw[N_BUCKETS]; >> + >> typedef struct domain { >> char *name; >> >> - context_map_node_t *raw_to_trans[N_BUCKETS]; >> - context_map_node_t *trans_to_raw[N_BUCKETS]; >> + context_map_node_t **raw_to_trans; >> + context_map_node_t **trans_to_raw; >> >> base_classification_t *base_classifications; >> word_group_t *groups; >> @@ -643,9 +646,11 @@ add_cache(domain_t *domain, char *raw, char *trans) { >> } >> >> log_debug(" add_cache (%s,%s)\n", raw, trans); >> + domain->raw_to_trans = table_raw_to_trans; >> if (add_to_hashtable(domain->raw_to_trans, map->raw, map) < 0) >> goto err; >> >> + domain->trans_to_raw = table_trans_to_raw; >> if (add_to_hashtable(domain->trans_to_raw, map->trans, map) < 0) >> goto err; >> >> @@ -721,7 +726,7 @@ static int read_translations(const char *filename); >> */ >> static int >> process_trans(char *buffer) { >> - static domain_t *domain; >> + domain_t *domain; > > Not really familiar with this code, but if you make this change, then > where is domain initialized prior to first use? The local domain will be initialized every time while calling the process_trans(): 784 if (!domain) { 785 domain = create_domain("Default"); but to be clear, the local "domain" definition should be: + domain_t *domain = NULL; > >> static word_group_t *group; >> static int base_classification; >> static int lineno = 0; > > And why is it ok for these to remain static? Oh, I need more test to verify those features, my testcase is only for default "setrans.conf". I'll update the patch after all test done. Thanks Wenzong > > >