On Wed, 2012-03-07 at 16:34 +0800, Harry Ciao wrote: > Comments embedded below. [lots of snipping throughout] > > @@ -530,6 +531,24 @@ int mls_compute_sid(struct context *scontext, > > r = hashtab_search(policydb.range_tr,&rtr); > > if (r) > > return mls_range_set(newcontext, r); > > + > > + cladatum = policydb.class_val_to_struct[tclass - 1]; > > + > > + switch (cladatum->default_range) { > > + case DEFAULT_SOURCE_LOW: > > + return mls_context_cpy_low(newcontext, scontext); > > + case DEFAULT_SOURCE_HIGH: > > + return mls_context_cpy_high(newcontext, scontext); > > + case DEFAULT_SOURCE_LOW_HIGH: > > + return mls_context_cpy(newcontext, scontext); > > + case DEFAULT_TARGET_LOW: > > + return mls_context_cpy_low(newcontext, tcontext); > > + case DEFAULT_TARGET_HIGH: > > + return mls_context_cpy_high(newcontext, tcontext); > > + case DEFAULT_TARGET_LOW_HIGH: > > + return mls_context_cpy(newcontext, tcontext); > > How about introducing a default case to take care of when default_range > is unset ? where mls_context_cpy_low(newcontext, scontext) could be > leveraged to handle such case. Notice the fallthrough. I added a few more lines of context. We do exactly what you ask for. If unset nothing changes from today. If set we get the defaults policy set. > > + } > > + > > /* Fallthrough */ > > case AVTAB_CHANGE: > > if ((tclass == policydb.process_class) || (sock == true)) /* Use the process MLS attributes. */ return mls_context_cpy(newcontext, scontext); else /* Use the process effective MLS attributes. */ return mls_context_cpy_low(newcontext, scontext); > > @@ -1450,17 +1456,25 @@ static int security_compute_sid(u32 ssid, > > break; > > } > > > > - /* Set the role and type to default values. */ > > - if ((tclass == policydb.process_class) || (sock == true)) { > > - /* Use the current role and type of process. */ > > + /* Set the role to default values. */ > > + if (cladatum->default_role == DEFAULT_SOURCE) { > > newcontext.role = scontext->role; > > - newcontext.type = scontext->type; > > + } else if (cladatum->default_role == DEFAULT_TARGET) { > > + newcontext.role = tcontext->role; > > } else { > > - /* Use the well-defined object role. */ > > - newcontext.role = OBJECT_R_VAL; > > + if ((tclass == policydb.process_class) || (sock == true)) > > + newcontext.role = scontext->role; > > + else > > + newcontext.role = OBJECT_R_VAL; > > OBJECT_R_VAL will be fallen back on when the default_role is unset, I > assume the above if-else condition could be eliminated if the > default_role for the process and various socket classes are specifically > defined as DEFAULT_SOURCE. Yes, it absolutely could. But we don't want to force a policy upgrade to upgrade the kernel. Thus we can't get rid of the legacy process/sock handling. I feel like any other option here would make the code worse, not better looking. > > + } > > + > > + /* Set the type to default values. */ > > + if ((tclass == policydb.process_class) || (sock == true)) > > + /* Use the type of process. */ > > + newcontext.type = scontext->type; > > + else > > If default_type would be employed then the process and all socket > classes won't have to be differentiated from other classes. Moreover, > the support for the "socket labeling" behavior would be obsolete now > that we have a much better solution and could be properly reverted. Yup. If it only weren't for those blasted users that don't update everything every time I might want them to *smile* Thanks for the comments! Though, I don't think there is a lot we can do to really make the code better while still supporting old systems. -Eric -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.