From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stas Sergeev Subject: Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler Date: Mon, 1 Feb 2016 02:45:16 +0300 Message-ID: <56AE9C8C.1070405@list.ru> References: <56AE3369.2090709@list.ru> <56AE3626.7080706@list.ru> <56AE4567.9000403@list.ru> <56AE69AD.6090005@list.ru> <56AE8C80.6030408@list.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Andy Lutomirski Cc: Linux kernel , Linux API , Ingo Molnar , Peter Zijlstra , Andrew Morton , Oleg Nesterov , Amanieu d'Antras , Richard Weinberger , Tejun Heo , "Kirill A. Shutemov" , Jason Low , Heinrich Schuchardt , Andrea Arcangeli , Konstantin Khlebnikov , Josh Triplett , "Eric W. Biederman" , Aleksa Sarai , Paul Moore , Palmer Dabbelt , Vladimir Davydov , Linus Torvalds List-Id: linux-api@vger.kernel.org 01.02.2016 01:44, Andy Lutomirski =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > On Sun, Jan 31, 2016 at 2:36 PM, Stas Sergeev wrote: >> 31.01.2016 23:11, Andy Lutomirski =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>> On Sun, Jan 31, 2016 at 12:08 PM, Stas Sergeev wrote= : >>>> 31.01.2016 22:03, Andy Lutomirski =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>>> Also, consider a use case like yours but with *two* contexts that= use >>>>> their own altstack. If you go to context A, enable sigaltstack, = get a >>>>> signal, temporarily disable, then swapcontext to B, which tries t= o >>>>> re-enable its own sigaltstack, then everything gets confusing wit= h >>>>> your patch, because, with your patch, the kernel is only tracking= one >>>>> temporarily disabled sigaltstack. >>>> Of course the good practice is to set the sigaltstack >>>> before creating the contexts. Then the above scenario >>>> should involve switching between 2 signal handlers to get >>>> into troubles. I think the scenario with switching between >>>> 2 signal handlers is very-very unrealistic. >>> Why is it so unrealistic? You're already using swapcontext, which >>> means you're doing something like userspace threads (although I >>> imagine that one of your thread-like things is DOS, but still), and= , >>> to me, that suggests that the kernel interface should be agnostic a= s >>> to how many thread-like thinks are alive. >> But you only get into troubles when you switch between 2 >> _active signal handlers_, rather than between 2 normal contexts, >> or between 2 normal context and 1 sighandler. >> So I am probably misunderstanding the scenario you describe. >> Without 2 sighandlers that are active at the same time and you >> switch between them, how would you get into troubles? >> You say "then swapcontext to B, which tries to re-enable its own >> sigaltstack" >> but there can be only one sigaltstack per thread, so I am quite >> sure by re-enabling "its own sigaltstack" it will still do the right >> thing. > As long as the kernel has a concept of a programmed but disabled > sigaltstack, something is confused when there is more than one > user-created inactive sigaltstack. I simply don't understand how can we have more than one sigaltstack per thread. Is this supported? If not then I don't expect the different non-sighandler user contexts trying to set up the different ones. So you are probably talking about 2 sighandlers switching between each other, right? And that case is likely broken anyway. > So I don't see why you want the > kernel to remember about disabled altstacks at all. 2 reasons: - Language-lawyering around POSIX - Consistently return oss->ss_flags when we are on a signal stack Restoring the old sas is not among the goals, but allowing the sighandler to freely install the new sas (as you suggest) is a clear contradiction to POSIX. So that's why you propose SS_FORCE, yes, but then the question: will _anyone_ use sigaltstack(SS_ONSTACK) in a sighandler without SS_FORCE? And the answer is likely "no". In which case your SS_FORCE erodes to "I run it from sighandler" but this info the kernel already knows. >> I don't think this is the problem because only the signal handler >> should re-enable the sigaltstack, and I don't think we really should >> switch between 2 active signal handlers. And even if we did, there >> can be only one sigaltstack per thread, so it will re-enable always >> the right stack (there is only one). > Why would there only be one per thread? If you mean every sighandler installs its own, then I think switching between such sighandlers is broken anyway. If you mean the non-sighandler contexts should install multiple sigaltstacks, then I don't think this is supported or can work. >>> ISTM it would be simpler if you did: >>> >>> sigaltstack(disable, force) >>> swapcontext() to context using sigaltstack >>> sigaltstack(set new altstack) >>> >>> and then later >>> >>> sigaltstack(disable, force) /* just in case. save old state, too.= */ >>> swapcontext() to context not using sigaltstack >>> sigaltstack(set new altstack) >> In the real world you don't even need sigaltstack(set new altstack) >> because uc_stack does this for you on rt_sigreturn. It is only my >> test-case that does so. > That's only the case if swapcontext is implemented using rt_sigreturn= =2E Is it? No, its when you use SA_SIGINFO with sigaction(). Then the sigaltstack will magically restore itself once you leave the sighandler. That's why I wouldn't suggest to ever modify the sas inside the sighandler the way you propose: it will simply not work, uc_stack will set it back. My re-enable trick is at least in agreement with uc_stack. >>> If it would be POSIX compliant to allow SS_DISABLE to work even if = on >>> the altstack even without a new flag (which is what you're >>> suggesting), then getting rid of the temporary in-kernel state woul= d >>> considerably simplify this patch series. Just skip the -EPERM chec= k >>> in the disable path. >> Yes, that's why I was suggesting to just remove the EPERM >> check initially. We can still do exactly that. The only problem I >> can see with removing EPERM is that it would be hard to emulate >> the old behaviour if need be. For example if glibc want to return >> EPERM behaviour, it will have problems doing so because oss->ss_flag= s >> doesn't say if we are on a sigaltstack and there is no other way >> to find out. > ...which is why I suggested SS_FORCE in the first place. I understand. With SS_FORCE there is no temptation to emulate the old behaviour, so there may be a fewer need to look into oss->sa_flags even when you return it an inconsistent ways. We probably should make a summary of our findings or they will be forgotten. So far I was pointing to a couple of minor problems with SS_FORCE: - bypasses overflow protection - prevents from asking the kernel if we are on sigaltstack or not =2E.. and a few that I consider more important: - does not bring any value other than to say "I am calling from sighand= ler" - allows the programmer to freely modify sas while later it will still be reset with uc_stack (and I've likely forgot some already) There are the upsides compared to just removing EPERM: - fewer need to look into oss->sa_flags, so its inconsistency became forgivable (have I missed something else? please fill in) There are the upsides compared to remembering the ss_flags: - simpler code - slightly better semantic wrt kernel threads (with my approach restoring the context on a different kernel thread, will require setting a separate sas there instead of restoring... but I am not sure this is a considerably bad semantic because whoever messes with kernel threads this way, should know how to propoerly set sigaltstacks) So from that summary I can agree that SS_FORCE may from some POV be better than simply removing EPERM, as they both share the downsides, with SS_FORCE providing minor advantages. But I still don't see it beating my new approach. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932954AbcAaXsC (ORCPT ); Sun, 31 Jan 2016 18:48:02 -0500 Received: from fallback2.mail.ru ([94.100.179.22]:59229 "EHLO fallback2.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932509AbcAaXr7 (ORCPT ); Sun, 31 Jan 2016 18:47:59 -0500 Subject: Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler To: Andy Lutomirski References: <56AE3369.2090709@list.ru> <56AE3626.7080706@list.ru> <56AE4567.9000403@list.ru> <56AE69AD.6090005@list.ru> <56AE8C80.6030408@list.ru> Cc: Linux kernel , Linux API , Ingo Molnar , Peter Zijlstra , Andrew Morton , Oleg Nesterov , "Amanieu d'Antras" , Richard Weinberger , Tejun Heo , "Kirill A. Shutemov" , Jason Low , Heinrich Schuchardt , Andrea Arcangeli , Konstantin Khlebnikov , Josh Triplett , "Eric W. Biederman" , Aleksa Sarai , Paul Moore , Palmer Dabbelt , Vladimir Davydov , Linus Torvalds From: Stas Sergeev Message-ID: <56AE9C8C.1070405@list.ru> Date: Mon, 1 Feb 2016 02:45:16 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Mras: Ok Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 01.02.2016 01:44, Andy Lutomirski пишет: > On Sun, Jan 31, 2016 at 2:36 PM, Stas Sergeev wrote: >> 31.01.2016 23:11, Andy Lutomirski пишет: >>> On Sun, Jan 31, 2016 at 12:08 PM, Stas Sergeev wrote: >>>> 31.01.2016 22:03, Andy Lutomirski пишет: >>>>> Also, consider a use case like yours but with *two* contexts that use >>>>> their own altstack. If you go to context A, enable sigaltstack, get a >>>>> signal, temporarily disable, then swapcontext to B, which tries to >>>>> re-enable its own sigaltstack, then everything gets confusing with >>>>> your patch, because, with your patch, the kernel is only tracking one >>>>> temporarily disabled sigaltstack. >>>> Of course the good practice is to set the sigaltstack >>>> before creating the contexts. Then the above scenario >>>> should involve switching between 2 signal handlers to get >>>> into troubles. I think the scenario with switching between >>>> 2 signal handlers is very-very unrealistic. >>> Why is it so unrealistic? You're already using swapcontext, which >>> means you're doing something like userspace threads (although I >>> imagine that one of your thread-like things is DOS, but still), and, >>> to me, that suggests that the kernel interface should be agnostic as >>> to how many thread-like thinks are alive. >> But you only get into troubles when you switch between 2 >> _active signal handlers_, rather than between 2 normal contexts, >> or between 2 normal context and 1 sighandler. >> So I am probably misunderstanding the scenario you describe. >> Without 2 sighandlers that are active at the same time and you >> switch between them, how would you get into troubles? >> You say "then swapcontext to B, which tries to re-enable its own >> sigaltstack" >> but there can be only one sigaltstack per thread, so I am quite >> sure by re-enabling "its own sigaltstack" it will still do the right >> thing. > As long as the kernel has a concept of a programmed but disabled > sigaltstack, something is confused when there is more than one > user-created inactive sigaltstack. I simply don't understand how can we have more than one sigaltstack per thread. Is this supported? If not then I don't expect the different non-sighandler user contexts trying to set up the different ones. So you are probably talking about 2 sighandlers switching between each other, right? And that case is likely broken anyway. > So I don't see why you want the > kernel to remember about disabled altstacks at all. 2 reasons: - Language-lawyering around POSIX - Consistently return oss->ss_flags when we are on a signal stack Restoring the old sas is not among the goals, but allowing the sighandler to freely install the new sas (as you suggest) is a clear contradiction to POSIX. So that's why you propose SS_FORCE, yes, but then the question: will _anyone_ use sigaltstack(SS_ONSTACK) in a sighandler without SS_FORCE? And the answer is likely "no". In which case your SS_FORCE erodes to "I run it from sighandler" but this info the kernel already knows. >> I don't think this is the problem because only the signal handler >> should re-enable the sigaltstack, and I don't think we really should >> switch between 2 active signal handlers. And even if we did, there >> can be only one sigaltstack per thread, so it will re-enable always >> the right stack (there is only one). > Why would there only be one per thread? If you mean every sighandler installs its own, then I think switching between such sighandlers is broken anyway. If you mean the non-sighandler contexts should install multiple sigaltstacks, then I don't think this is supported or can work. >>> ISTM it would be simpler if you did: >>> >>> sigaltstack(disable, force) >>> swapcontext() to context using sigaltstack >>> sigaltstack(set new altstack) >>> >>> and then later >>> >>> sigaltstack(disable, force) /* just in case. save old state, too. */ >>> swapcontext() to context not using sigaltstack >>> sigaltstack(set new altstack) >> In the real world you don't even need sigaltstack(set new altstack) >> because uc_stack does this for you on rt_sigreturn. It is only my >> test-case that does so. > That's only the case if swapcontext is implemented using rt_sigreturn. Is it? No, its when you use SA_SIGINFO with sigaction(). Then the sigaltstack will magically restore itself once you leave the sighandler. That's why I wouldn't suggest to ever modify the sas inside the sighandler the way you propose: it will simply not work, uc_stack will set it back. My re-enable trick is at least in agreement with uc_stack. >>> If it would be POSIX compliant to allow SS_DISABLE to work even if on >>> the altstack even without a new flag (which is what you're >>> suggesting), then getting rid of the temporary in-kernel state would >>> considerably simplify this patch series. Just skip the -EPERM check >>> in the disable path. >> Yes, that's why I was suggesting to just remove the EPERM >> check initially. We can still do exactly that. The only problem I >> can see with removing EPERM is that it would be hard to emulate >> the old behaviour if need be. For example if glibc want to return >> EPERM behaviour, it will have problems doing so because oss->ss_flags >> doesn't say if we are on a sigaltstack and there is no other way >> to find out. > ...which is why I suggested SS_FORCE in the first place. I understand. With SS_FORCE there is no temptation to emulate the old behaviour, so there may be a fewer need to look into oss->sa_flags even when you return it an inconsistent ways. We probably should make a summary of our findings or they will be forgotten. So far I was pointing to a couple of minor problems with SS_FORCE: - bypasses overflow protection - prevents from asking the kernel if we are on sigaltstack or not ... and a few that I consider more important: - does not bring any value other than to say "I am calling from sighandler" - allows the programmer to freely modify sas while later it will still be reset with uc_stack (and I've likely forgot some already) There are the upsides compared to just removing EPERM: - fewer need to look into oss->sa_flags, so its inconsistency became forgivable (have I missed something else? please fill in) There are the upsides compared to remembering the ss_flags: - simpler code - slightly better semantic wrt kernel threads (with my approach restoring the context on a different kernel thread, will require setting a separate sas there instead of restoring... but I am not sure this is a considerably bad semantic because whoever messes with kernel threads this way, should know how to propoerly set sigaltstacks) So from that summary I can agree that SS_FORCE may from some POV be better than simply removing EPERM, as they both share the downsides, with SS_FORCE providing minor advantages. But I still don't see it beating my new approach.