From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleg Nesterov Subject: Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler Date: Mon, 1 Feb 2016 18:09:58 +0100 Message-ID: <20160201170958.GA20735@redhat.com> References: <56AE3369.2090709@list.ru> <56AE3626.7080706@list.ru> <20160201160625.GA18276@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20160201160625.GA18276-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Stas Sergeev Cc: Linux kernel , linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andy Lutomirski , Ingo Molnar , Peter Zijlstra , Andrew Morton , 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 List-Id: linux-api@vger.kernel.org On 02/01, Oleg Nesterov wrote: > > > + onsigstack = on_sig_stack(sp); > > + if (ss_size == 0) { > > + switch (ss_flags) { > > + case 0: > > + error = -EPERM; > > + if (onsigstack) > > + goto out; > > + current->sas_ss_sp = 0; > > + current->sas_ss_size = 0; > > + current->sas_ss_flags = SS_DISABLE; > > + break; > > + case SS_ONSTACK: > > + /* re-enable previously disabled sas */ > > + error = -EINVAL; > > + if (current->sas_ss_size == 0) > > + goto out; > > + break; > > + default: > > + break; > > + } > > and iiuc the "default" case allows you to write SS_DISABLE into ->sas_ss_flags > even if on_sig_stack(). > > So the sequence is > > // running on alt stack > > sigaltstack(SS_DISABLE); > > temporary_run_on_another_stack(); > > sigaltstack(SS_ONSTACK); > > and SS_DISABLE saves us from another SA_ONSTACK signal, right? > > But afaics it can only help after we change the stack. Suppose that SA_ONSTACK signal > comess before temporary_run_on_another_stack(). get_sigframe() should be fine after > your changes (afaics), it won't pick the alt stack after SS_DISABLE. > > However, unless I missed something save_altstack_ex() will record SS_ONSTACK in > uc_stack->ss_flags, and after return from signal handler restore_altstack() will > enable alt stack again? OK, I didn't notice you modified save_altstack_ex() to use ->sas_ss_flags instead of sas_ss_flags()... still doesn't look right, in this case restore_altstack() will not restore sas_ss_size/sas_ss_sp and they can be changed by signal handler. Anyway, whatever I missed I agree with Andy, SS_FORCE looks simpler and better to me. Oleg. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754122AbcBARKH (ORCPT ); Mon, 1 Feb 2016 12:10:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36515 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753249AbcBARKF (ORCPT ); Mon, 1 Feb 2016 12:10:05 -0500 Date: Mon, 1 Feb 2016 18:09:58 +0100 From: Oleg Nesterov To: Stas Sergeev Cc: Linux kernel , linux-api@vger.kernel.org, Andy Lutomirski , Ingo Molnar , Peter Zijlstra , Andrew Morton , "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 Subject: Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler Message-ID: <20160201170958.GA20735@redhat.com> References: <56AE3369.2090709@list.ru> <56AE3626.7080706@list.ru> <20160201160625.GA18276@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160201160625.GA18276@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/01, Oleg Nesterov wrote: > > > + onsigstack = on_sig_stack(sp); > > + if (ss_size == 0) { > > + switch (ss_flags) { > > + case 0: > > + error = -EPERM; > > + if (onsigstack) > > + goto out; > > + current->sas_ss_sp = 0; > > + current->sas_ss_size = 0; > > + current->sas_ss_flags = SS_DISABLE; > > + break; > > + case SS_ONSTACK: > > + /* re-enable previously disabled sas */ > > + error = -EINVAL; > > + if (current->sas_ss_size == 0) > > + goto out; > > + break; > > + default: > > + break; > > + } > > and iiuc the "default" case allows you to write SS_DISABLE into ->sas_ss_flags > even if on_sig_stack(). > > So the sequence is > > // running on alt stack > > sigaltstack(SS_DISABLE); > > temporary_run_on_another_stack(); > > sigaltstack(SS_ONSTACK); > > and SS_DISABLE saves us from another SA_ONSTACK signal, right? > > But afaics it can only help after we change the stack. Suppose that SA_ONSTACK signal > comess before temporary_run_on_another_stack(). get_sigframe() should be fine after > your changes (afaics), it won't pick the alt stack after SS_DISABLE. > > However, unless I missed something save_altstack_ex() will record SS_ONSTACK in > uc_stack->ss_flags, and after return from signal handler restore_altstack() will > enable alt stack again? OK, I didn't notice you modified save_altstack_ex() to use ->sas_ss_flags instead of sas_ss_flags()... still doesn't look right, in this case restore_altstack() will not restore sas_ss_size/sas_ss_sp and they can be changed by signal handler. Anyway, whatever I missed I agree with Andy, SS_FORCE looks simpler and better to me. Oleg.