On Thu, Jul 6, 2017 at 9:34 AM, Linus Torvalds <torva...@linux-foundation.org> wrote: > On Wed, Jul 5, 2017 at 9:32 PM, Kees Cook <keesc...@chromium.org> wrote: >> In an attempt to provide sensible rlimit defaults for setuid execs, this >> inherits the namespace's init rlimits: > > Yeah, so I have to admit to hating this patch.
Yup! Totally fine. I just wanted to demonstrate the idea with some actual code. > I'd much rather see something like > > (a) minimal: just use our existing default stack (and stack _only_) > limit value for suid binaries that actually get extra permissions: { > _STK_LIM, RLIM_INFINITY }. This would look a lot like the existing patch; it'd just not copy the init process rlimits. > (c) perhaps encourage people to annotate their suid binaries with > initial resource requirements (and for stack, I mean the existing > GNU_STACK ELF annotation in particular). > > For an example of (a), that existing _STK_LIM define is what the > kernel defaults to, and it's a 8MB stack. And looking at my Fedora > install, I see that the default user rlimit is 8MB for the stack. > [...] > Is that just coincidence, or is that just a sign of "nobody ever even > modifies the default value"? So (a) feels like "nobody really cares, > and 8MB is fine, and nobody even bothers changing it - just do the > minimal thing". I would be terrified to see a setuid binary that needed >8MB stack. :P > And (c) would be the sane option, and what we already do for things > like GNU_STACK to enable/disable executable stacks. It really feels > like allowing the GNU_STACK segment to contain stack rlimit override > information would be the perfect tool for binaries to say "Yeah, I > need more stack than _STK_LIM". > > So I see many different approaches (that could be combined: I like > combining (a) and (c), for example), and absolutely none of them > involve the random "take some values from init". > > And yes, a large part of this may be that I no longer feel like I can > trust "init" to do the sane thing. You all presumably know why. Heh. Yeah, I think a+c could make sense. "if not GNU_STACK program header or 0 MemSiz GNU_STACK program header, use _STK_LIM, otherwise use GNU_STACK MemSiz". I'll send a patch... -Kees -- Kees Cook Pixel Security