4 ms·

I don't really understand the rationale for changing the Linux kernel at all here; yes, this is yet another footgun (tm), but this is purely a userspace problem

by tremon 5y ago

I don't really understand the rationale for changing the Linux kernel at all here; yes, this is yet another footgun (tm), but this is purely a userspace problem. Especially given the examples regarding backwards compatibility given in the comments, I would expect a long-lasting userspace fix to come in the form of a new, better libc API. Not by prohibiting certain behaviour from the kernel-side of things. That said, why is the obvious fix not simply "always populate argv[0] with pathname if called with NULL argv"?

deleted 5y ago
[deleted]
MatmaRex 5y ago
Your question is answered in the article: > Given the code we've found that depends on NULL argv, I think we likely can't make the change outright, so we're down this weird rabbit hole of trying to reject what we can and create work-around behaviors for the cases that currently exist.
- tremon 5y ago
  It's not clear from that quote if they're talking about code calling execve(,NULL,) or a program expecting main() to be entered with argv[0]==NULL. (edit: found an answer here: https://lwn.net/ml/linux-kernel/5e963fab-88d4-2039-1cf4-6661e9bd16b@dereferenced.org/ https://lwn.net/ml/linux-kernel/5e963fab-88d4-2039-1cf4-6661...) multicall programs will try to use argv[0] and might crash in this scenario. If we're going to fake an argv, I guess we should try to do it right.
  - formerly_proven 5y ago
    (multicall = things like busybox which look at argv[0] to determine what program they should act like)
- Animats 5y ago
  Right, backwards compatibility with existing bugs. It really isn't a kernel problem at all, of course. The POSIX spec says "The first argument, argv[0], is required and must contain the name of the executable file for the new process image."[1] Linux is not fully POSIX-compliant, but it generally follows the POSIX standard for kernel calls defined in POSIX. FreeBSD's man page says "At least one argument must be present in the array; by custom, the first element should be the name of the executed program (for example, the last component of path)."[2] So, "must", compatible with the POSIX spec. QNX implements the POSIX API, pretty much per the spec. The QNX spec says: "The value in argv[0] must point to a filename that's associated with the process being started."[4] So, "must", again compatible with the spec. The Linux man page says, unfortunately, "By convention, the first of these strings (i.e., argv[0]) should contain the filename associated with the file being executed."[3] That's the problem. POSIX says "must" but the Linux page only says "should". That was a mistake. One somewhat kludgy fix would be to disallow argv[0] == NULL && envp[0] != NULL. That allows launching a program with no args at all, which might be used somewhere in some ancient program, while disallowing launch with no command line args but with environment args, which is probably a bug. (I liked the QNX approach to program loading. The kernel isn't involved. The "exec" functions link to a shared object, which requests memory and does the loading using regular I/O operations. The kernel contains no loader and is thus immune to problems from malformed executables.) [1] https://support.sas.com/documentation/onlinedoc/sasc/doc750/html/lr2/zid-7281.htm https://support.sas.com/documentation/onlinedoc/sasc/doc750/... [2] https://manpages.debian.org/unstable/freebsd-manpages/execve.2freebsd.en.html https://manpages.debian.org/unstable/freebsd-manpages/execve... [3] https://www.man7.org/linux/man-pages/man2/execve.2.html https://www.man7.org/linux/man-pages/man2/execve.2.html [4] https://www.qnx.com/developers/docs/6.4.0/neutrino/lib_ref/e/execve.html https://www.qnx.com/developers/docs/6.4.0/neutrino/lib_ref/e...
  - hvdijk 5y ago
    That isn't the POSIX spec.
    - Animats 5y ago
      You're right. This is (I think) the actual spec.[1] The other one is SAS's reference which claims to describe the POSIX spec. The actual spec says "should". Bleah. "The arguments represented by arg0,... are pointers to null-terminated character strings. These strings shall constitute the argument list available to the new process image. The list is terminated by a null pointer. The argument arg0 should point to a filename string that is associated with the process being started by one of the exec functions." "The argument argv is an array of character pointers to null-terminated strings. The application shall ensure that the last member of this array is a null pointer. These strings shall constitute the argument list available to the new process image. The value in argv[0] should point to a filename string that is associated with the process being started by one of the exec functions." [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html https://pubs.opengroup.org/onlinepubs/9699919799/functions/e...
akira2501 5y ago
That was my issue with the article as well: "Fixing this issue is a simple matter of making pkexec check that argc is at least one. But there are surely other programs out there containing similar assumptions." That's _not_ an assumption. That's just a bug, in a setuid binary.
- sedatk 5y ago
  It doesn't matter if it's a bug in the program or not. "We don't break the userspace" --Linus Torvalds
  - akira2501 5y ago
    I'm describing an issue with the article, not with the kernel development ideology. What I don't understand is why Corbett decided to write this up as an "assumption" rather than a "bug" which it clearly is.
    - zerocrates 5y ago
      It's both, what's the problem with what he wrote? It's a bug that results from the developers incorrectly assuming that argc is always at least 1.
asveikau 5y ago
I don't see why pushing this out to glibc would help. Glibc would then face the same question about whether or not some programs rely on current behavior and would break.
- mst 5y ago
  Linus' views on backcompat and glibc's have, let us say, not been entirely aligned over the years. If the change fails at the kernel level but succeeds at the glibc level that might still be a net win overall, and given Ariadne as "instigator" here I'd be hopeful that she could convince musl to come along for the ride if that was how it had to happen. (assuming she thought that was a good idea in the first place, of course, I'm exploring counterfactuals here)
  - isomel 5y ago
    But a "fix" in libc wouldn't help for the security risk because an attacker could still emit the system call directly. (Unless I misunderstood what the fix would be)
    - asveikau 5y ago
      At first I imagine the libc killing the process if argv is empty. Otoh if you want libc to be possibly a little too clever, perhaps it could make an effort to try to discover a sensible value for argv[0] based on looking around /proc etc to find the path to the binary. That seems like way too much complexity and failure points for a few mostly inconsequential corner cases.
      - thingsgoup 5y ago
        If libc isn’t linked into the process to begin with then none of that matters.
        asveikau 5y ago
        Many languages that might want to bypass libc would also bounds check argv, so ... Maybe it doesn't matter in a few more senses than that ...
ajnin 5y ago
polkit is a freedesktop.org project, convincing Linus to break userspace is an easier deal than getting them to admit fault. (joking)

by tremon 5y ago

deleted 5y ago

[deleted]

MatmaRex 5y ago

Your question is answered in the article: > Given the code we've found that depends on NULL argv, I think we likely can't make the change outright, so we're down this weird rabbit hole of trying to reject what we can and create work-around behaviors for the cases that currently exist.

tremon 5y ago
It's not clear from that quote if they're talking about code calling execve(,NULL,) or a program expecting main() to be entered with argv[0]==NULL. (edit: found an answer here: https://lwn.net/ml/linux-kernel/5e963fab-88d4-2039-1cf4-6661e9bd16b@dereferenced.org/ https://lwn.net/ml/linux-kernel/5e963fab-88d4-2039-1cf4-6661...) multicall programs will try to use argv[0] and might crash in this scenario. If we're going to fake an argv, I guess we should try to do it right.
- formerly_proven 5y ago
  (multicall = things like busybox which look at argv[0] to determine what program they should act like)
Animats 5y ago
Right, backwards compatibility with existing bugs. It really isn't a kernel problem at all, of course. The POSIX spec says "The first argument, argv[0], is required and must contain the name of the executable file for the new process image."[1] Linux is not fully POSIX-compliant, but it generally follows the POSIX standard for kernel calls defined in POSIX. FreeBSD's man page says "At least one argument must be present in the array; by custom, the first element should be the name of the executed program (for example, the last component of path)."[2] So, "must", compatible with the POSIX spec. QNX implements the POSIX API, pretty much per the spec. The QNX spec says: "The value in argv[0] must point to a filename that's associated with the process being started."[4] So, "must", again compatible with the spec. The Linux man page says, unfortunately, "By convention, the first of these strings (i.e., argv[0]) should contain the filename associated with the file being executed."[3] That's the problem. POSIX says "must" but the Linux page only says "should". That was a mistake. One somewhat kludgy fix would be to disallow argv[0] == NULL && envp[0] != NULL. That allows launching a program with no args at all, which might be used somewhere in some ancient program, while disallowing launch with no command line args but with environment args, which is probably a bug. (I liked the QNX approach to program loading. The kernel isn't involved. The "exec" functions link to a shared object, which requests memory and does the loading using regular I/O operations. The kernel contains no loader and is thus immune to problems from malformed executables.) [1] https://support.sas.com/documentation/onlinedoc/sasc/doc750/html/lr2/zid-7281.htm https://support.sas.com/documentation/onlinedoc/sasc/doc750/... [2] https://manpages.debian.org/unstable/freebsd-manpages/execve.2freebsd.en.html https://manpages.debian.org/unstable/freebsd-manpages/execve... [3] https://www.man7.org/linux/man-pages/man2/execve.2.html https://www.man7.org/linux/man-pages/man2/execve.2.html [4] https://www.qnx.com/developers/docs/6.4.0/neutrino/lib_ref/e/execve.html https://www.qnx.com/developers/docs/6.4.0/neutrino/lib_ref/e...
- hvdijk 5y ago
  That isn't the POSIX spec.
  - Animats 5y ago
    You're right. This is (I think) the actual spec.[1] The other one is SAS's reference which claims to describe the POSIX spec. The actual spec says "should". Bleah. "The arguments represented by arg0,... are pointers to null-terminated character strings. These strings shall constitute the argument list available to the new process image. The list is terminated by a null pointer. The argument arg0 should point to a filename string that is associated with the process being started by one of the exec functions." "The argument argv is an array of character pointers to null-terminated strings. The application shall ensure that the last member of this array is a null pointer. These strings shall constitute the argument list available to the new process image. The value in argv[0] should point to a filename string that is associated with the process being started by one of the exec functions." [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html https://pubs.opengroup.org/onlinepubs/9699919799/functions/e...

akira2501 5y ago

That was my issue with the article as well: "Fixing this issue is a simple matter of making pkexec check that argc is at least one. But there are surely other programs out there containing similar assumptions." That's _not_ an assumption. That's just a bug, in a setuid binary.

sedatk 5y ago
It doesn't matter if it's a bug in the program or not. "We don't break the userspace" --Linus Torvalds
- akira2501 5y ago
  I'm describing an issue with the article, not with the kernel development ideology. What I don't understand is why Corbett decided to write this up as an "assumption" rather than a "bug" which it clearly is.
  - zerocrates 5y ago
    It's both, what's the problem with what he wrote? It's a bug that results from the developers incorrectly assuming that argc is always at least 1.

asveikau 5y ago

I don't see why pushing this out to glibc would help. Glibc would then face the same question about whether or not some programs rely on current behavior and would break.

mst 5y ago
Linus' views on backcompat and glibc's have, let us say, not been entirely aligned over the years. If the change fails at the kernel level but succeeds at the glibc level that might still be a net win overall, and given Ariadne as "instigator" here I'd be hopeful that she could convince musl to come along for the ride if that was how it had to happen. (assuming she thought that was a good idea in the first place, of course, I'm exploring counterfactuals here)
- isomel 5y ago
  But a "fix" in libc wouldn't help for the security risk because an attacker could still emit the system call directly. (Unless I misunderstood what the fix would be)
  - asveikau 5y ago
    At first I imagine the libc killing the process if argv is empty. Otoh if you want libc to be possibly a little too clever, perhaps it could make an effort to try to discover a sensible value for argv[0] based on looking around /proc etc to find the path to the binary. That seems like way too much complexity and failure points for a few mostly inconsequential corner cases.
    - thingsgoup 5y ago
      If libc isn’t linked into the process to begin with then none of that matters.
      - asveikau 5y ago
        Many languages that might want to bypass libc would also bounds check argv, so ... Maybe it doesn't matter in a few more senses than that ...

ajnin 5y ago

polkit is a freedesktop.org project, convincing Linus to break userspace is an easier deal than getting them to admit fault. (joking)