Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > ( > > On Sun, Apr 15, 2018 at 8:56 AM, Eric W. Biederman > <ebiederm@xxxxxxxxxxxx> wrote: >> >> Would you consider the patchset below for -rc2? > > Ugh. The point of this series is to squash the potential for regressions even from the weird broken code that fills in fields for si_code 0 that do not match SI_USER. The copy_siginfo_to_user32 never handled that so we don't need to worry about that. All of the signals that abuse si_code 0 go through force_sig_info so signalfd can not catch them. Which means that only copy_siginfo_to_user needs to be worried about. Last time I was benchmarking I could not see a difference between copying the entire siginfo and only a small part so I don't see the point of optimizing now. For an actual cleanup I intend to go farther than you are proposing and convert everthing to the set of helper functions I have added to kernel/signal.c force_sig_fault, force_sig_mceerr, force_sig_bnderr, force_sig_pkuerr. When I am done there will be maybe 5 instances of clear_siginfo outside of those helper functions. At which point I won't care if we remove clear_siginfo and just use memset. That is a real improvement that looks something like: "105 files changed, 933 insertions(+), 1698 deletions(-)" That is my end goal with all of this. You complained to me about regressions and you are right with the current handling of FPE_FIXME TRAP_FIXME the code is not bug compatible with old versions of linux, and people have noticed. What this patchset represents is the bare minimum needed to convert copy_siginfo_to_user to a copy_to_user, and remove the possibility of regressions. To that end I need to ensure every struct siginfo in the entire kernel is memset to 0. A structure initializer is absolutely not enough. So I don't mind people thinking clear_siginfo is necessary. To that end clear_siginfo where it does not take the size of struct siginfo as a parameter is much less suceptible to typos, and allows me to ensure every instance of struct siginfo is fully initialized. I fully intend to remove practically every instance of struct siginfo from the architecture specific code, and use the aforementioned helpers. The trouble is that some of the architecture code need refactoring for that to happen. Even your proposed init_siginfo can't be widely used as a common pattern is to fill in siginfo partially in the code with si_code and si_signo being filled in almost last. So in short. I intend to remove most of these clear_siginfo calls when I remove the siginfos later. This series is focused on making copy_siginfo_to_user simple enough we don't need to use siginfo_layout, and thus can be fully backwards compatibile with ourselves and we won't need to worry about regressions. I have aimed to keep this simple enough we can merge this after -rc1 because we don't want regressions. Eric ps. I intend to place this change first in my series wether or not it makes it into -rc2 so that I can be certain we remove any possible regressions in behavior on the buggy architectures. Then I can take my time and ensure the non-trivial changes of refactoring etc are done carefully so I don't introduce other bugs. I need that so I can sleep at night. pps. I can look at some of your other suggestions but cleverness leads to regressions, and if you are going to complain at me harshly when I have been being careful and taking things seriously I am not particularly willing to take unnecessary chances.
