On Mon, Oct 24, 2016 at 06:11:04PM -0500, Eric W. Biederman wrote: > Kees Cook <keescook@xxxxxxxxxxxx> writes: > > > On Mon, Oct 24, 2016 at 1:29 PM, Cyrill Gorcunov <gorcunov@xxxxxxxxx> wrote: > >> On Mon, Oct 24, 2016 at 02:01:30PM -0500, Eric W. Biederman wrote: > >>> So I am probably going to tweak the !mm case so that instead of failing > >>> we perform the old capable check in that case. That seems the mot > >>> certain way to avoid regressions. With that said, why is exit_code > >>> behind a ptrace_may_access permission check? > >> > >> Yes, this would be great! And as to @exit_code I think better ask > >> Kees, CC'ed. > > > > My concern was that this was an exposure in the sense that it is > > internal program state that isn't visible through other means (without > > being the parent, for example). Under the ptrace check, it has an > > equivalency that seemed correct at the time. > > > > As already covered, I'd agree: it looks like ce99dd5fd5f6 accidentally > > added a dependency on task->mm where it didn't before. That section of > > logic was entirely around dumpability, not an mm existing. It should > > be "EPERM if mm and dumpable != SUID_DUMP_USER". > > > > That said, I'd also agree that ptrace against no mm is crazy (though I > > suppose that should return EINVAL or ESRCH or something), so perhaps a > > better access control on @exit_code is needed here. > > I traced down the original logic of why we had that dumpable > variable, and it was ancient conservative on my part when we started > using the ptrace permission checks for proc files. > > That same conservatism has resulted in the regression under > discussion. > > Given that we already have a very full set of permission checks > separate from dumpable in ptrace_may_access I think it makes sense > to simply ignore dumpable when there is no mm. > AKA: > mm = task->mm; > if (mm && > ((get_dumpable(mm) != SUID_DUMP_USER) && > !ptrace_has_cap(mm->user_ns, mode))) > return -EPERM; > > Because while it has been used for other things dumpable is > fundamentally about do you have permission to read the mm. > So there is no real point in permission checks that protect > the mm if the mm has gone away. > > It also looks like I may need to update the check that sets > PT_PTRACE_CAP to look at mm->user_ns as well. Thanks a lot for informative explanations, Eric and Kees! Eric, if you make some patch please ping me to test it. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
