On Mon, 10 Sep 2018, Schaufler, Casey wrote: > It you're going to call __ptrace_access_check(),
I guess you mean __ptrace_may_access() here. > which already includes an LSM hook, it makes a whole lot of sense to > make that the path for doing any module specific checks. It seems wrong > to disable the LSM hook there, then turn around and introduce a new one > that does the check you just disabled. The patches I had proposed > created a new LSM hook because there was not path to an existing hook. > With your addition of __ptrace_access_check() that is no longer an issue > once any locking problems are resolved. Rather than use a new hook, the > existing ptrace hooks ought to work just fine, and any new checks can be > added in a new module that has its own ptrace_access_check hook. Sorry for being dense, but what exactly are you proposing here then? This patch (v4 and v5) explicitly avoids calling out to ptrace_has_cap() (which calls out to LSM through has_ns_capability_*() -> security_capable()) in PTRACE_MODE_IBPB case, exactly to avoid locking issues with security modules; there are known callchains that lead to deadlock. With the same reasoning, security_ptrace_access_check() call is avoided, only there is no know particular callchain that'd lead to a lock being taken, but noone has done such audit (yet), as it's all hidden behind LSM callbacks. So please tell me what exactly you'd like to see changed in the IBPB patch and why exactly, I am not seeing it yet. Thanks, -- Jiri Kosina SUSE Labs
