Re: [PATCH] Split bprm_apply_creds into two functions

[Stephen Smalley]

On Wed, 2004-12-15 at 17:52, Chris Wright wrote:
> I don't like this approach.  The whole point is to ensure safety, and
> avoid races that have been found in the past.  This gives a new interface
> that could be easily used under the wrong conditions, and breaking
> the interface into two pieces looks kinda hackish.  Is there no other
> solution?  I looked at this once before and wondered why task_unlock()
> is needed to call avc_audit?  audit should be as lock friendly as printk
> IMO, and I don't recall seeing any deadlock after short review of it.
> But I didn't get much beyond that.  Is it all the flushing that can't
> hold task_lock?

avc_audit() used to always call get_task_mm() when fetching the mm for
use in determining the executable, which was producing deadlock when the
caller held task lock.  However, I changed it to only use get_task_mm
when acting on a task other than current, since we can safely access
current->mm, which eliminated the deadlock for the checks in
bprm_apply_creds.  That is why Serge's patch folds
avc_has_perm_noaudit()+avc_audit() pairs down to avc_has_perm() and
keeps them in bprm_apply_creds.  Hence, avc_audit is no longer a
concern.

The concern is with lock nesting for the flushing code, e.g. call to
force_sig_specific and signal inheritance flush would nest siglock under
task lock and call to flush_unauthorized_files would nest file_list_lock
and file_lock under task lock.  That code didn't used to be called under
task lock prior to the reworking of compute_creds by Andy Lutomirski,
and when compute_creds was overhauled, there was some concern expressed
by Andrew Morton about the lock nesting, iirc.

Note that the flushing code isn't relying on the safety flags computed
earlier by unsafe_exec, so it really doesn't need the task lock for that
purpose.

-- 
Stephen Smalley <sds@xxxxxxxxxxxxxx>
National Security Agency

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/