Rethinking the Stack Clash fix

Posted Jul 13, 2017 23:28 UTC (Thu) by foom (subscriber, #14868)
In reply to: Rethinking the Stack Clash fix by nix
Parent article: Rethinking the Stack Clash fix

Um...how come nobody has noted that this new patch totally breaks the userspace API promises?

There is an API for getting the max argument size, sysconf(_SC_ARG_MAX). Documented in man sysconf:

       ARG_MAX - _SC_ARG_MAX
              The maximum length of the arguments to the exec(3) family of functions.  Must not be less than _POSIX_ARG_MAX (4096).

Implemented in glibc as:

    case _SC_ARG_MAX:
      /* Use getrlimit to get the stack limit.  */
      if (__getrlimit (RLIMIT_STACK, &rlimit) == 0)
        return MAX (legacy_ARG_MAX, rlimit.rlim_cur / 4);

      return legacy_ARG_MAX;

With this kernel patch, that is now a complete lie, and any program depending on it in order to decide whether the arguments can be passed on the commandline (vs., say, in a response file) are now broken. The clang compiler is one such program I know of, but I'm sure there are others...

WTF?

Rethinking the Stack Clash fix

Posted Jul 14, 2017 4:49 UTC (Fri) by areilly (subscriber, #87829) [Link] (1 responses)

Pretty sure that clang is not usually installed setuid. My reading of the article suggested that those are the only ones getting the proposed limit.
I'm not sure how well an suid limit like this would interact with something like xargs, that presumably knows similar things about allowable argument sizes. It would seem to require xargs to be aware of the setuid-nature of its client program.

Rethinking the Stack Clash fix

Posted Jul 14, 2017 11:20 UTC (Fri) by foom (subscriber, #14868) [Link]

The article discussed and links to (see part that says "This patch has been merged") a patch that applies a 6MB ARG_MAX maximal limit /regardless/ of suid or not. That results in sysconf lying if the stack rlimit has been set to 24MB or higher.

Rethinking the Stack Clash fix

Posted Jul 14, 2017 12:23 UTC (Fri) by joey (guest, #328) [Link] (6 responses)

The canonical example of using ARG_MAX is xargs. This is going to cause massive breakage if it slips in.

Rethinking the Stack Clash fix

Posted Jul 14, 2017 12:39 UTC (Fri) by jchaxby (subscriber, #63942) [Link] (2 responses)

And find ... -exec {} +

And I'm pretty sure there are other perfectly legitimate uses, as nix suggests, of very large arg sizes.

"Argument list too long"

Posted Jul 27, 2017 16:46 UTC (Thu) by geuder (subscriber, #62854) [Link] (1 responses)

In an up-to-date OpenSUSE 42.2 I got "Argument list too long" when using "xargs -0" today for the first time after I don't know how many years. So I remembered this discussion.

I have not tweaked ulimit in anyway, the stack size is 8192.

Luckily "find ... -exec ... {} +" worked without the error. Was just a bad old habit to use xargs in this case, where exec + was possible, too. But there might be some more legitimate use cases of "xargs -0"

Unluckily that means that I have not digged any deeper whether it was really a stack clash fix that introduced the "argument list too long" problem. Would be be quite weird coincidence if it weren't.

"Argument list too long"

Posted Jul 27, 2017 18:11 UTC (Thu) by mbunkus (subscriber, #87248) [Link]

> But there might be some more legitimate use cases of "xargs -0"

Parallel execution, for example: `… | xargs -0 --max-args=1 --max-procs=$(getconf _NPROCESSORS_ONLN) the-program-to-parallelize`

Rethinking the Stack Clash fix

Posted Jul 14, 2017 15:06 UTC (Fri) by corbet (editor, #1) [Link] (2 responses)

Perhaps, but remember that, unless you've tweaked the stack limit yourself, the limit was (and remains) 2MB. Are there really "massive" numbers of users who have changed the stack limit for the purposes of enabling a much larger args array?

Rethinking the Stack Clash fix

Posted Jul 15, 2017 0:46 UTC (Sat) by foom (subscriber, #14868) [Link]

I suspect it's not a massive number of people with large or unlimited stack size, unless some distro has done so by default.

But, once you have increased it for *any* purpose, this kernel change will break programs attempting to choose the right number of args to pass on the command line via calling sysconf. You don't need to be actively requiring or even desiring a large args array...

Rethinking the Stack Clash fix

Posted Jul 17, 2017 14:21 UTC (Mon) by NightMonkey (subscriber, #23051) [Link]

Maybe those same users who pushed for the ext4 "largefile" limit increase to 2 billion files? ;)

Rethinking the Stack Clash fix

Posted Jul 14, 2017 18:03 UTC (Fri) by Frogging101 (guest, #113180) [Link]

Methinks someone should reply and point out that this is broken.

I would do it but I don't want to butt in without really knowing what I'm talking about.