Bug 205831
Summary: | Invalid use of x86 INT instruction should generate SIGILL, not SIGSEGV | ||
---|---|---|---|
Product: | Platform Specific/Hardware | Reporter: | Zack Weinberg (zackw) |
Component: | x86-64 | Assignee: | platform_x86_64 (platform_x86_64) |
Status: | RESOLVED WILL_NOT_FIX | ||
Severity: | normal | CC: | bp, luto |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 5.3.0 | Subsystem: | |
Regression: | No | Bisected commit-id: | |
Attachments: | test program |
Description
Zack Weinberg
2019-12-10 21:18:15 UTC
I don't think we can just go changing signal numbers, and I also disagree that SIGILL makes any sense here. Unfortunately, the design and documentation is nonsense, but the fields in the ABI to look at are in the ucontext: both the trap number and error code are there. I can see an argument that this can't be changed now for backward compatibility's sake. I don't agree with it - I would argue that any program that would be broken by the proposed change is already buggy - but I see where you're coming from. I do _not_ see why you think SIGILL doesn't make sense. This is not an invalid access to memory, it's an attempt to execute an instruction that is forbidden to user space processes. That is exactly what SIGILL/ILL_PRVOPC is supposed to mean. "It's a #GP fault, so we report it as a SIGSEGV" only makes sense if you insist on wearing x86-colored glasses. Also, needing to look at the ucontext to understand what's going on, when the kernel _could_ be exposing all of the necessary information in the siginfo, is a shitty thing to do to user space programmers. ucontext is both too poorly documented, and way too machine-specific, to make people bother with if there's any alternative. On 2019-12-11 10:51, bugzilla-daemon@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=205831 > > --- Comment #2 from Zack Weinberg (zackw@panix.com) --- > I can see an argument that this can't be changed now for backward > compatibility's sake. I don't agree with it - I would argue that any program > that would be broken by the proposed change is already buggy - but I see > where > you're coming from. > > I do _not_ see why you think SIGILL doesn't make sense. This is not an > invalid > access to memory, it's an attempt to execute an instruction that is forbidden > to user space processes. That is exactly what SIGILL/ILL_PRVOPC is supposed > to > mean. "It's a #GP fault, so we report it as a SIGSEGV" only makes sense if > you > insist on wearing x86-colored glasses. > > Also, needing to look at the ucontext to understand what's going on, when the > kernel _could_ be exposing all of the necessary information in the siginfo, > is > a shitty thing to do to user space programmers. ucontext is both too poorly > documented, and way too machine-specific, to make people bother with if > there's > any alternative. > Uhm, INT is an x86 instruction, and it is NOT an illegal instruction; some INT instructions are in fact permitted from user space. So the idea that this is somehow "x86-colored glasses" is silly. So, 1. First of all, we don't change a 28-year-old ABI which would break userspace. 2. SIGILL, on x86, typically doesn't provide information about the faulting instruction as it is not available to the kernel without getting it out of user space memory anyway. (In fact, x86 always reports ILL_ILLOPN 3. "Too poorly documented and way too machine-specific" -- how on Earth do you expect to get information about an x86-specific instruction without machine-specific knowledge? However, please feel free to contribute documentation; it is always appreciated. -hpa INT is, in fact, highly unusual among x86 instructions in that the exception that the CPU generates is highly informative. So, in theory, the kernel *could* detect that a #GP fault is due to INT and do something clever with it, but I also don't see the point at all. Also, making INT work differently from every other #GP seems confusing at best. Can someone with appropriate permissions please close this bug? . > Uhm, INT is an x86 instruction, and it is NOT an illegal instruction; some > INT > instructions are in fact permitted from user space. So the idea that this is > somehow "x86-colored glasses" is silly. I do not find this line of reasoning persuasive, but I also don't care enough to argue about it further. However ... > please feel free to contribute documentation; it is always appreciated. ... yeah, I think I _will_ write a patch for the signal(7) manpage that explains that for historical reasons, on some architectures, some operations that are forbidden to user space are reported using SIGSEGV instead of SIGINT and the siginfo_t is useless. In the BUGS section. Y'all OK with that? (In reply to Zack Weinberg from comment #6) > > Uhm, INT is an x86 instruction, and it is NOT an illegal instruction; some > > INT > > instructions are in fact permitted from user space. So the idea that this > is > > somehow "x86-colored glasses" is silly. > > I do not find this line of reasoning persuasive, but I also don't care > enough to argue about it further. However ... > > > please feel free to contribute documentation; it is always appreciated. > > ... yeah, I think I _will_ write a patch for the signal(7) manpage that > explains that for historical reasons, on some architectures, some operations > that are forbidden to user space are reported using SIGSEGV instead of > SIGINT and the siginfo_t is useless. In the BUGS section. Y'all OK with > that? If you call it "historical reasons", I will NAK that patch. Let's be crystal clear here. The CPU generates one type of fault (#UD) for unrecognized instructions and generates a different type of fault (#GP) for many other things, including various cases of insufficient privilege. Other causes of #GP include dereferencing a bad pointer for certain values of "bad". Sending SIGILL for #UD makes sense. Sending SIGILL for #GP, in general, would be utter nonsense. So you're welcome to explain that SIGSEGV also includes certain privilege violations, and documenting the error_code and trap_nr fields would be nice, but the only historical thing here is the odd location of the error_code and trap_nr fields. |