On Wed, Oct 19, 2016 at 10:55 AM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: > Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes: > >> On Wed, Oct 19, 2016 at 10:29 AM, Jann Horn <jann@xxxxxxxxx> wrote: >>> On Wed, Oct 19, 2016 at 11:52:50AM -0500, Eric W. Biederman wrote: >>>> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes: >>>> > Simply ptrace yourself, exec the >>>> > program, and then dump the program out. A program that really wants >>>> > to be unreadable should have a stub: the stub is setuid and readable, >>>> > but all the stub does is to exec the real program, and the real >>>> > program should have mode 0500 or similar. >>>> > >>>> > ISTM the "right" check would be to enforce that the program's new >>>> > creds can read the program, but that will break backwards >>>> > compatibility. >>>> >>>> Last I looked I had the impression that exec of a setuid program kills >>>> the ptrace. >>>> >>>> If we are talking about a exec of a simple unreadable executable (aka >>>> something that sets undumpable but is not setuid or setgid). Then I >>>> agree it should break the ptrace as well and since those programs are as >>>> rare as hens teeth I don't see any problem with changing the ptrace behavior >>>> in that case. >>> >>> Nope. check_unsafe_exec() sets LSM_UNSAFE_* flags in bprm->unsafe, and then >>> the flags are checked by the LSMs and cap_bprm_set_creds() in commoncap.c. >>> cap_bprm_set_creds() just degrades the execution to a non-setuid-ish one, >>> and e.g. ptracers stay attached. >> >> I think you're right. I ought to be completely sure because I rewrote >> that code back in 2005 or so back when I thought kernel programming >> was only for the cool kids. It was probably my first kernel patch >> ever and it closed an awkward-to-exploit root hole. But it's been a >> while. (Too bad my second (IIRC) kernel patch was more mundane and >> fixed the mute button on "new" Lenovo X60-era laptops and spend >> several years in limbo...) > > Ah yes and this is only a problem if the ptracer does not have > CAP_SYS_PTRACE. > > If the tracer does not have sufficient permissions any opinions on > failing the exec or kicking out the ptracer? I am leaning towards failing > the exec as it is more obvious if someone cares. Dropping the ptracer > could be a major mystery. I would suggest leaving it alone. Changing it could break enough things that a sysctl would be needed, and I just don't see how this is a significant issue, especially since it's been insecure forever. Anyone who cares should do the stub executable trick: /sbin/foo: 04755, literally just does execve("/sbin/foo-helper"); /sbin/foo-helper: 0500. --Andy -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>