Re: [PATCH v2 2/2] rusage: allow 64-bit times ru_utime/ru_stime
Ingo Molnar writes: > * Eric W. Biederman wrote: > >> Ingo Molnar writes: >> >> > * Eric W. Biederman wrote: >> > >> >> The trouble with attributes is that means you can't filter your system >> >> call arguments with seccomp. [...] >> > >> > There's nothing keeping seccomp from securely fetching those arguments and >> > extending filtering to them as well ... >> > >> > Allowing that would make sense for a lot of other system calls as >> > well. >> >> Possibly. The challenge is that if the fetch for the kernel to use >> those arguments is different from the fetch of seccomp to test those >> arguments you have a time of test vs time of use race. > > Those fetched values should obviously then be used to call permitted > system calls. Agreed. To my knowledge no one has figured out how to make that work yet. For the most part it has been unnecessary. >> Given the location of the seccomp hook at the kernel user space border >> there is no easy way for seccomp to share the fetch with the system >> call itself. >> >> So I don't see how seccomp could perform the fetch securely. > > Looks like more of a seccomp mis-design/mis-implementation than some > fundamental > problem. Frankly. Given that there are some very good solutions in other operating systems, I think the misdesign is in unix/linux not providing a good answer to what to do when you need more than 6 arguments to a system call. > Mis-designed security features should not hinder system call design. I certainly agree that seccomp should not be the sole reason for not doing something. However there are lots of reasons to avoid extensibility in general. Excess extensibility has been the cause of more than one security issue. Lots of flexibility comes at the price of lots of conditional execution which tends to explode the test matrix of possibilities to test, with the result that some combinations are never thought about or tested because they don't make sense to combine. Then someone with mischievious intent see that combination and thinks what happens when I do this. Further that conditional execution can frequently be the cause of slow code as well. So while there are many nice features of tagged values. I don't think they are a general solution. The lack of seccomp support (today) is just one downside among many. I do think it would be nice to have a general pattern for those system calls that require extensibility. My gut feel says something like the L4 pseudo registers (to give a maxium request size) combined with something like netlink encoding would make a very nice template for making fast and flexible system calls. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] rusage: allow 64-bit times ru_utime/ru_stime
Ingo Molnar writes: > * Eric W. Biederman wrote: > >> The trouble with attributes is that means you can't filter your system >> call arguments with seccomp. [...] > > There's nothing keeping seccomp from securely fetching those arguments and > extending filtering to them as well ... > > Allowing that would make sense for a lot of other system calls as > well. Possibly. The challenge is that if the fetch for the kernel to use those arguments is different from the fetch of seccomp to test those arguments you have a time of test vs time of use race. Given the location of the seccomp hook at the kernel user space border there is no easy way for seccomp to share the fetch with the system call itself. So I don't see how seccomp could perform the fetch securely. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] rusage: allow 64-bit times ru_utime/ru_stime
Ingo Molnar writes: > * Arnd Bergmann wrote: > >> However, the other question that has to be asked then is whether >> there is anything wrong with wait4()/waitid() and getrusuage() that >> we want to change beyond the time value passing. We have >> answered a similar question with 'yes' for stat(), which has led >> to the introduction of statx(), > > So we are thinking about adding wait5() in essence, right? > One thing we might want to look into whether the wait4() and waitid() ABIs > could > be 'merged', by making wait4() essentially a natural special case of > waitid(). Essentially waitid(2) not waitid(3) has already seen this merger. In that there is nothing to wait for that you can not already expression with waitid. status vs siginfo is a little different but the information is encoded in both. And waitid(2) optionally returns a struct rusage. > This would mean that the only new system call we'd have to add is waitid2() > in > essence, which would solve both the rusage layout problem and would offer a > unified ABI. > > If that makes sense (it might not!!), then I'd also modernize waitid2() by > making > it attribute structure based, have a length field and make the ABI extensible > from > now on going forward without having to introduce a new syscall variant every > time > we come up with something new... The only part where something is not parameterized in waitid is with the return of rusage. What to wait for takes an explicit type parameter. What is being returned in siginfo returns an si_code to describe how to decode it. If it weren't for the zombie being gone after waitid returns I don't think it would make any sense to combine getrusage and waitid together at all. > I.e. how the perf syscall does ABI extensions: we've had dozens of ABI > extensions, > some of them pretty complex, and not a single time did we have to modify > glibc and > tooling was able to adapt quickly yet in a both backwards and forwards > compatible > fashion. > > Another, simpler example is the new sys_sched_setattr() syscall, that too is > using > the perf_copy_attr() ABI method, via sched_copy_attr(). (With a minor > compatibility quirk of SCHED_ATTR_SIZE_VER0 that a new wait ABI wouldn't have > to > do - i.e. it could be made even simpler.) > > This way we only have: > > SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr, > unsigned int, flags) > > But even 'pid' and 'flags' could have been part of the attribute, i.e. one we > pick > up an attribute structure from user-space we can have really low argument > count > system calls. This also concentrates all the compat concerns into handling > the > attribute structure properly - no weird per-arch artifacts and quirks with > 4-5-6 > system call arguments. The trouble with attributes is that means you can't filter your system call arguments with seccomp. Which most of the time is a pretty big downside. >From what I have seen the only truly interesting case for extending waitid is something file descriptor based so the parent/child relationship is not necessary to wait for a process to terminate. As for getrusage. If a sane union of the rusage limits and cgroups or something like cgroups could be devised. That would be ideal. Of course except for the memory cgroups the similarity to the resource usage measurments and limits really isn't there. So I don't know if merging them would be a real possibility. So I suspect the simplest thing to do would be to set a flag in the idtype member of waitid that says give me rusage64 and then we would be done. Alternately we could use the low bits of the resource usage pointer. Assuming we don't want to introduce another syscall that is. I really don't see much incremental extensibility potential in the wait or rusage interface right now. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW][PATCH 02/22] signal/alpha: Use force_sig_fault where appropriate
Filling in struct siginfo before calling force_sig_info a tedious and error prone process, where once in a great while the wrong fields are filled out, and siginfo has been inconsistently cleared. Simplify this process by using the helper force_sig_fault. Which takes as a parameters all of the information it needs, ensures all of the fiddly bits of filling in struct siginfo are done properly and then calls force_sig_info. In short about a 5 line reduction in code for every time force_sig_info is called, which makes the calling function clearer. Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: linux-alpha@vger.kernel.org Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com> --- arch/alpha/kernel/traps.c | 9 + arch/alpha/mm/fault.c | 15 ++- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c index 106a1692fca0..bc9627698796 100644 --- a/arch/alpha/kernel/traps.c +++ b/arch/alpha/kernel/traps.c @@ -400,16 +400,9 @@ do_entIF(unsigned long type, struct pt_regs *regs) asmlinkage void do_entDbg(struct pt_regs *regs) { - siginfo_t info; - die_if_kernel("Instruction fault", regs, 0, NULL); - clear_siginfo(); - info.si_signo = SIGILL; - info.si_errno = 0; - info.si_code = ILL_ILLOPC; - info.si_addr = (void __user *) regs->pc; - force_sig_info(SIGILL, , current); + force_sig_fault(SIGILL, ILL_ILLOPC, (void __user *)regs->pc, 0, current); } diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c index 7f2202a9f50a..de2bd217adad 100644 --- a/arch/alpha/mm/fault.c +++ b/arch/alpha/mm/fault.c @@ -88,11 +88,8 @@ do_page_fault(unsigned long address, unsigned long mmcsr, struct mm_struct *mm = current->mm; const struct exception_table_entry *fixup; int fault, si_code = SEGV_MAPERR; - siginfo_t info; unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; - clear_siginfo(); - /* As of EV6, a load into $31/$f31 is a prefetch, and never faults (or is suppressed by the PALcode). Support that for older CPUs by ignoring such an instruction. */ @@ -223,21 +220,13 @@ do_page_fault(unsigned long address, unsigned long mmcsr, up_read(>mmap_sem); /* Send a sigbus, regardless of whether we were in kernel or user mode. */ - info.si_signo = SIGBUS; - info.si_errno = 0; - info.si_code = BUS_ADRERR; - info.si_addr = (void __user *) address; - force_sig_info(SIGBUS, , current); + force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *) address, 0, current); if (!user_mode(regs)) goto no_context; return; do_sigsegv: - info.si_signo = SIGSEGV; - info.si_errno = 0; - info.si_code = si_code; - info.si_addr = (void __user *) address; - force_sig_info(SIGSEGV, , current); + force_sig_fault(SIGSEGV, si_code, (void __user *) address, 0, current); return; #ifdef CONFIG_ALPHA_LARGE_VMALLOC -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW][PATCH 01/22] signal/alpha: Use send_sig_fault where appropriate
Filling in struct siginfo before calling send_sig_info a tedious and error prone process, where once in a great while the wrong fields are filled out, and siginfo has been inconsistently cleared. Simplify this process by using the helper send_sig_fault. Which takes as a parameters all of the information it needs, ensures all of the fiddly bits of filling in struct siginfo are done properly and then calls send_sig_info. In short about a 5 line reduction in code for every time send_sig_info is called, which makes the calling function clearer. Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: linux-alpha@vger.kernel.org Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com> --- arch/alpha/kernel/osf_sys.c | 10 ++- arch/alpha/kernel/signal.c | 22 +++--- arch/alpha/kernel/traps.c | 71 +++-- 3 files changed, 24 insertions(+), 79 deletions(-) diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c index bb3619118926..6e921754c8fc 100644 --- a/arch/alpha/kernel/osf_sys.c +++ b/arch/alpha/kernel/osf_sys.c @@ -871,7 +871,6 @@ SYSCALL_DEFINE5(osf_setsysinfo, unsigned long, op, void __user *, buffer, send a signal. Old exceptions are not signaled. */ fex = (exc >> IEEE_STATUS_TO_EXCSUM_SHIFT) & swcr; if (fex) { - siginfo_t info; int si_code = FPE_FLTUNK; if (fex & IEEE_TRAP_ENABLE_DNO) si_code = FPE_FLTUND; @@ -881,12 +880,9 @@ SYSCALL_DEFINE5(osf_setsysinfo, unsigned long, op, void __user *, buffer, if (fex & IEEE_TRAP_ENABLE_DZE) si_code = FPE_FLTDIV; if (fex & IEEE_TRAP_ENABLE_INV) si_code = FPE_FLTINV; - clear_siginfo(); - info.si_signo = SIGFPE; - info.si_errno = 0; - info.si_code = si_code; - info.si_addr = NULL; /* FIXME */ - send_sig_info(SIGFPE, , current); + send_sig_fault(SIGFPE, si_code, + (void __user *)NULL, /* FIXME */ + 0, current); } return 0; } diff --git a/arch/alpha/kernel/signal.c b/arch/alpha/kernel/signal.c index cd306e602313..8c0c4ee0be6e 100644 --- a/arch/alpha/kernel/signal.c +++ b/arch/alpha/kernel/signal.c @@ -219,15 +219,8 @@ do_sigreturn(struct sigcontext __user *sc) /* Send SIGTRAP if we're single-stepping: */ if (ptrace_cancel_bpt (current)) { - siginfo_t info; - - clear_siginfo(); - info.si_signo = SIGTRAP; - info.si_errno = 0; - info.si_code = TRAP_BRKPT; - info.si_addr = (void __user *) regs->pc; - info.si_trapno = 0; - send_sig_info(SIGTRAP, , current); + send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc, 0, + current); } return; @@ -254,15 +247,8 @@ do_rt_sigreturn(struct rt_sigframe __user *frame) /* Send SIGTRAP if we're single-stepping: */ if (ptrace_cancel_bpt (current)) { - siginfo_t info; - - clear_siginfo(); - info.si_signo = SIGTRAP; - info.si_errno = 0; - info.si_code = TRAP_BRKPT; - info.si_addr = (void __user *) regs->pc; - info.si_trapno = 0; - send_sig_info(SIGTRAP, , current); + send_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *) regs->pc, 0, + current); } return; diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c index 242c83d86ace..106a1692fca0 100644 --- a/arch/alpha/kernel/traps.c +++ b/arch/alpha/kernel/traps.c @@ -213,7 +213,6 @@ do_entArith(unsigned long summary, unsigned long write_mask, struct pt_regs *regs) { long si_code = FPE_FLTINV; - siginfo_t info; if (summary & 1) { /* Software-completion summary bit is set, so try to @@ -228,21 +227,14 @@ do_entArith(unsigned long summary, unsigned long write_mask, } die_if_kernel("Arithmetic fault", regs, 0, NULL); - clear_siginfo(); - info.si_signo = SIGFPE; - info.si_errno = 0; - info.si_code = si_code; - info.si_addr = (void __user *) regs->pc; - send_sig_info(SIGFPE, , current); + send_sig_fault(SIGFPE, si_code, (void __user *) regs->pc, 0, current); } asmlinkage void do_entIF(unsigned long type, struct pt_regs *regs) { - siginfo_t info; int signo, code; - clear_siginfo(); if ((regs->ps & ~IPL_MAX) == 0) {
[REVIEW][PATCH 15/17] signal: Add TRAP_UNK si_code for undiagnosted trap exceptions
Both powerpc and alpha have cases where they wronly set si_code to 0 in combination with SIGTRAP and don't mean SI_USER. About half the time this is because the architecture can not report accurately what kind of trap exception triggered the trap exception. The other half the time it looks like no one has bothered to figure out an appropriate si_code. For the cases where the architecture does not have enough information or is too lazy to figure out exactly what kind of trap exception it is define TRAP_UNK. Cc: linux-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Cc: linux-alpha@vger.kernel.org Cc: linuxppc-...@lists.ozlabs.org Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com> --- arch/x86/kernel/signal_compat.c| 2 +- include/uapi/asm-generic/siginfo.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c index 14c057f29979..9ccbf0576cd0 100644 --- a/arch/x86/kernel/signal_compat.c +++ b/arch/x86/kernel/signal_compat.c @@ -29,7 +29,7 @@ static inline void signal_compat_build_tests(void) BUILD_BUG_ON(NSIGFPE != 15); BUILD_BUG_ON(NSIGSEGV != 7); BUILD_BUG_ON(NSIGBUS != 5); - BUILD_BUG_ON(NSIGTRAP != 4); + BUILD_BUG_ON(NSIGTRAP != 5); BUILD_BUG_ON(NSIGCHLD != 6); BUILD_BUG_ON(NSIGSYS != 1); diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index 558b902f18d4..80e2a7227205 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -249,7 +249,8 @@ typedef struct siginfo { #define TRAP_TRACE 2 /* process trace trap */ #define TRAP_BRANCH 3 /* process taken branch trap */ #define TRAP_HWBKPT 4 /* hardware breakpoint/watchpoint */ -#define NSIGTRAP 4 +#define TRAP_UNK 5 /* undiagnosed trap */ +#define NSIGTRAP 5 /* * There is an additional set of SIGTRAP si_codes used by ptrace -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW][PATCH 11/17] signal/alpha: Replace FPE_FIXME with FPE_FLTUNK
Using an si_code of 0 that aliases with SI_USER is clearly the wrong thing todo, and causes problems in interesting ways. The newly defined FPE_FLTUNK semantically appears to fit the bill so use it instead. Given recent experience in this area odds are it will not break anything. Fixing it removes a hazard to kernel maintenance. Cc: Helge Deller <del...@gmx.de> Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: Matt Turner <matts...@gmail.com> Cc: linux-alpha@vger.kernel.org History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Fixes: 0a635c7a84cf ("Fill in siginfo_t.") Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com> --- arch/alpha/include/uapi/asm/siginfo.h | 7 --- arch/alpha/kernel/osf_sys.c | 2 +- arch/alpha/kernel/traps.c | 2 +- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/arch/alpha/include/uapi/asm/siginfo.h b/arch/alpha/include/uapi/asm/siginfo.h index 0cf3b527b274..3ebbb1e17902 100644 --- a/arch/alpha/include/uapi/asm/siginfo.h +++ b/arch/alpha/include/uapi/asm/siginfo.h @@ -7,13 +7,6 @@ #include -/* - * SIGFPE si_codes - */ -#ifdef __KERNEL__ -#define FPE_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - /* * SIGTRAP si_codes */ diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c index f5f154942aab..bb3619118926 100644 --- a/arch/alpha/kernel/osf_sys.c +++ b/arch/alpha/kernel/osf_sys.c @@ -872,7 +872,7 @@ SYSCALL_DEFINE5(osf_setsysinfo, unsigned long, op, void __user *, buffer, fex = (exc >> IEEE_STATUS_TO_EXCSUM_SHIFT) & swcr; if (fex) { siginfo_t info; - int si_code = FPE_FIXME; + int si_code = FPE_FLTUNK; if (fex & IEEE_TRAP_ENABLE_DNO) si_code = FPE_FLTUND; if (fex & IEEE_TRAP_ENABLE_INE) si_code = FPE_FLTRES; diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c index 91636765dd6d..422b676b28f2 100644 --- a/arch/alpha/kernel/traps.c +++ b/arch/alpha/kernel/traps.c @@ -328,7 +328,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) break; case GEN_ROPRAND: signo = SIGFPE; - code = FPE_FIXME; + code = FPE_FLTUNK; break; case GEN_DECOVF: -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW][PATCH 16/17] signal/alpha: Replace TRAP_FIXME with TRAP_UNK
Using an si_code of 0 that aliases with SI_USER is clearly the wrong thing to do, and causes problems in interesting ways. For it really is not clear to me if using TRAP_UNK bugcheck or the default case of gentrap is really the best way to handle things. There is certainly enough information that that a more specific si_code could potentially be used. That said TRAP_UNK is definitely an improvement over 0 as it removes the ambiguiuty of what si_code of 0 with SIGTRAP means on alpha. Recent history suggests no actually cares about crazy corner cases of the kernel behavior like this so I don't expect any regressions from changing this. However if something does happen this change is easy to revert. Cc: Helge Deller <del...@gmx.de> Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: Matt Turner <matts...@gmail.com> Cc: linux-alpha@vger.kernel.org Fixes: 0a635c7a84cf ("Fill in siginfo_t.") History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com> --- arch/alpha/include/uapi/asm/siginfo.h | 7 --- arch/alpha/kernel/traps.c | 4 ++-- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/arch/alpha/include/uapi/asm/siginfo.h b/arch/alpha/include/uapi/asm/siginfo.h index 3ebbb1e17902..db3f0138536f 100644 --- a/arch/alpha/include/uapi/asm/siginfo.h +++ b/arch/alpha/include/uapi/asm/siginfo.h @@ -7,11 +7,4 @@ #include -/* - * SIGTRAP si_codes - */ -#ifdef __KERNEL__ -#define TRAP_FIXME 0 /* Broken dup of SI_USER */ -#endif /* __KERNEL__ */ - #endif diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c index 422b676b28f2..242c83d86ace 100644 --- a/arch/alpha/kernel/traps.c +++ b/arch/alpha/kernel/traps.c @@ -288,7 +288,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) case 1: /* bugcheck */ info.si_signo = SIGTRAP; info.si_errno = 0; - info.si_code = TRAP_FIXME; + info.si_code = TRAP_UNK; info.si_addr = (void __user *) regs->pc; info.si_trapno = 0; send_sig_info(SIGTRAP, , current); @@ -350,7 +350,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) case GEN_SUBRNG7: default: signo = SIGTRAP; - code = TRAP_FIXME; + code = TRAP_UNK; break; } -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] y2038: rusage: use __kernel_old_timeval for process times
Paul Eggertwrites: > On 11/27/2017 09:00 AM, Arnd Bergmann wrote: >> b) Extend the approach taken by the x32 ABI, and use the 64-bit >> native structure layout for rusage on all architectures with new >> system calls that is otherwise compatible. A possible problem here >> is that we end up with incompatible definitions of rusage between >> /usr/include/linux/resource.h and /usr/include/bits/resource.h >> >> c) Change the definition of struct rusage to be independent of >> time_t. This is the easiest change, as it does not involve new system >> call entry points, but it has the risk of introducing compile-time >> incompatibilities with user space sources that rely on the type >> of ru_utime and ru_stime. >> >> I'm picking approch c) for its simplicity, but I'd like to hear from >> others whether they would prefer a different approach. > > (c) would break programs like GNU Emacs, which copy ru_utime and ru_stime > members into struct timeval variables. > > All in all, (b) sounds like it would be better for programs using glibc, as > it's > more compatible with what POSIX apps expect. Though I'm not sure what problems > are meant by "possible ... incompatible definitions"; perhaps you could > elaborate. getrusage is posix and I believe the use of struct timeval is posix as well. So getrusage(3) the libc definition and that defintion must struct timeval or the implementation will be non-conforming and it won't be just emacs we need to worry about. The practical question is what do we provide to userspace so that it can implement a conforming getrusage? A 32bit time_t based struct timeval is good for durations up to 136 years or so. Which strongly suggests the range is large enough, except for some crazy massively multi-threaded application. And anything off the charts cpu hungry at this point I expect will be 64bit. It is possible to get a 128 way system with one thread on each core and consume 100% of the core for a bit over a year to max out getrusage. So I do think in the long run we care about increasing the size of time_t here. Last I checked applications doing things like that were 64bit in the year 2000. Given that userspace is going to be seeing the larger struct rusage in any event my inclination for long term maintainability would be to introduce the new syscall and have the current one called oldgetrusage on 32bit architectures. Then we won't have to worry about what weird things glibc will do when translating the data, and we can handle applications with crazy (but possible) runtimes. Which inclines me to (b) as well. As for (a) does anyone have a need for process acounting at nsec granularity? Unless we can get that for free that just seems like overpromising and a waist to have so much fine granularity. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] signal/alpha: Document a conflict with SI_USER for SIGTRAP
Setting si_code to __SI_FAULT results in a userspace seeing an si_code of 0. This is the same si_code as SI_USER. Posix and common sense requires that SI_USER not be a signal specific si_code. As such this use of 0 for the si_code is a pretty horribly broken ABI. Given that alpha is on it's last legs I don't know that it is worth fixing this, but it is worth documenting what is going on so that no one decides to copy this bad decision. This was introduced during the 2.5 development cycle so this mess has had a long time for people to be able to depend upon it. v2: Added FPE_FIXME for alpha as Helge Deller <del...@gmx.de> pointed out with his alternate patch one of the cases is SIGFPE not SIGTRAP. Cc: Helge Deller <del...@gmx.de> Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: Matt Turner <matts...@gmail.com> Cc: linux-alpha@vger.kernel.org History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Ref: 0a635c7a84cf ("Fill in siginfo_t.") Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com> --- arch/alpha/include/uapi/asm/siginfo.h | 14 ++ arch/alpha/kernel/traps.c | 6 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/arch/alpha/include/uapi/asm/siginfo.h b/arch/alpha/include/uapi/asm/siginfo.h index 9822362a8424..972f547d9e41 100644 --- a/arch/alpha/include/uapi/asm/siginfo.h +++ b/arch/alpha/include/uapi/asm/siginfo.h @@ -6,4 +6,18 @@ #include +/* + * SIGFPE si_codes + */ +#ifdef __KERNEL__ +#define FPE_FIXME (__SI_FAULT|0) /* Broken dup of SI_USER */ +#endif /* __KERNEL__ */ + +/* + * SIGTRAP si_codes + */ +#ifdef __KERNEL__ +#define TRAP_FIXME (__SI_FAULT|0) /* Broken dup of SI_USER */ +#endif /* __KERNEL__ */ + #endif diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c index 65bb102d985b..e94f4b73ac04 100644 --- a/arch/alpha/kernel/traps.c +++ b/arch/alpha/kernel/traps.c @@ -278,7 +278,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) case 1: /* bugcheck */ info.si_signo = SIGTRAP; info.si_errno = 0; - info.si_code = __SI_FAULT; + info.si_code = TRAP_FIXME; info.si_addr = (void __user *) regs->pc; info.si_trapno = 0; send_sig_info(SIGTRAP, , current); @@ -318,7 +318,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) break; case GEN_ROPRAND: signo = SIGFPE; - code = __SI_FAULT; + code = FPE_FIXME; break; case GEN_DECOVF: @@ -340,7 +340,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) case GEN_SUBRNG7: default: signo = SIGTRAP; - code = __SI_FAULT; + code = TRAP_FIXME; break; } -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP
Michael Cree <mc...@orcon.net.nz> writes: > On Fri, Jul 14, 2017 at 05:59:06AM -0500, Eric W. Biederman wrote: >> in which cases the oddities will happen let alone test them. Plus at >> least for ia64 and alpha those architectures don't appear to be >> receiving updates for new syscalls, and no new hardware is being built >> so I don't know how much longer they will last. >> >> That is building for alpha gives: >> > CALL >> > /home/eric/projects/linux/linux-exit-cleanups/scripts/checksyscalls.sh >> > :1239:2: warning: #warning syscall seccomp not implemented [-Wcpp] >> > :1248:2: warning: #warning syscall bpf not implemented [-Wcpp] >> > :1299:2: warning: #warning syscall userfaultfd not implemented >> > [-Wcpp] >> > :1302:2: warning: #warning syscall membarrier not implemented >> > [-Wcpp] >> > :1305:2: warning: #warning syscall mlock2 not implemented [-Wcpp] >> > :1308:2: warning: #warning syscall copy_file_range not implemented >> > [-Wcpp] >> > :1311:2: warning: #warning syscall preadv2 not implemented [-Wcpp] >> > :1314:2: warning: #warning syscall pwritev2 not implemented [-Wcpp] >> > :1317:2: warning: #warning syscall pkey_mprotect not implemented >> > [-Wcpp] >> > :1320:2: warning: #warning syscall pkey_alloc not implemented >> > [-Wcpp] >> > :1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp] >> > :1326:2: warning: #warning syscall statx not implemented [-Wcpp] > > Patches to wire up most of those syscalls on Alpha was posted recently > along with others to fix module loading, etc., but unfortunately they > do not appear to have been applied during the merge window... Odd. In general wiring up system calls is something that can even happen in -rc2 or -rc3 as system calls get added in -rc1 and then the architectures have a chance to send in the small patches wiring the up. At least that is how I understand the usual rules for being a non-x86 architecture. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] signal/alpha: Document a conflict with SI_USER for SIGTRAP
Helge Deller <del...@gmx.de> writes: > * Eric W. Biederman <ebied...@xmission.com>: >> Setting si_code to __SI_FAULT results in a userspace seeing >> an si_code of 0. This is the same si_code as SI_USER. Posix >> and common sense requires that SI_USER not be a signal specific >> si_code. As such this use of 0 for the si_code is a pretty >> horribly broken ABI. >> >> Given that alpha is on it's last legs I don't know that it is worth >> fixing this, but it is worth documenting what is going on so that >> no one decides to copy this bad decision. > > The ABI was already broken, so IMHO I think it's better to somehow "fix" > it instead. Agreed, alpha and some other architectures are already aged, > but nevertheless most of them build in debian-ports. Below is a > suggested fix which reuses/misuses other existing trap codes instead. The conflict with SI_USER is real. I don't know that we have any actual applications that are suffering. I simply don't have the architecture specific references handy to know in which cases the oddities will happen let alone test them. Plus at least for ia64 and alpha those architectures don't appear to be receiving updates for new syscalls, and no new hardware is being built so I don't know how much longer they will last. That is building for alpha gives: > CALL > /home/eric/projects/linux/linux-exit-cleanups/scripts/checksyscalls.sh > :1239:2: warning: #warning syscall seccomp not implemented [-Wcpp] > :1248:2: warning: #warning syscall bpf not implemented [-Wcpp] > :1299:2: warning: #warning syscall userfaultfd not implemented [-Wcpp] > :1302:2: warning: #warning syscall membarrier not implemented [-Wcpp] > :1305:2: warning: #warning syscall mlock2 not implemented [-Wcpp] > :1308:2: warning: #warning syscall copy_file_range not implemented > [-Wcpp] > :1311:2: warning: #warning syscall preadv2 not implemented [-Wcpp] > :1314:2: warning: #warning syscall pwritev2 not implemented [-Wcpp] > :1317:2: warning: #warning syscall pkey_mprotect not implemented > [-Wcpp] > :1320:2: warning: #warning syscall pkey_alloc not implemented [-Wcpp] > :1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp] > :1326:2: warning: #warning syscall statx not implemented [-Wcpp] But a real fix like you have shown if you can get it approved by the alpha architecture maintainers I am more than happy to see, and would very much prefer. You certainly noticed that the middle case was SIGFPE when I missed that. Unfortunately I don't currently have the competence to judge the effects of your patch. Eric > Helge > > Signed-off-by: Helge Deller <del...@gmx.de> > > diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c > index 65bb102..2ed37dd 100644 > --- a/arch/alpha/kernel/traps.c > +++ b/arch/alpha/kernel/traps.c > @@ -278,7 +278,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) > case 1: /* bugcheck */ > info.si_signo = SIGTRAP; > info.si_errno = 0; > - info.si_code = __SI_FAULT; > + info.si_code = TRAP_HWBKPT; > info.si_addr = (void __user *) regs->pc; > info.si_trapno = 0; > send_sig_info(SIGTRAP, , current); > @@ -318,7 +318,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) > break; > case GEN_ROPRAND: > signo = SIGFPE; > - code = __SI_FAULT; > + code = FPE_FLTSUB; > break; > > case GEN_DECOVF: > @@ -340,7 +340,7 @@ do_entIF(unsigned long type, struct pt_regs *regs) > case GEN_SUBRNG7: > default: > signo = SIGTRAP; > - code = __SI_FAULT; > + code = TRAP_HWBKPT; > break; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html