From: Christophe Leroy <christophe.leroy@csgroup.eu> To: "Eric W. Biederman" <ebiederm@xmission.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Michael Ellerman <mpe@ellerman.id.au>, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v2 3/5] signal: Add unsafe_copy_siginfo_to_user() Date: Fri, 10 Sep 2021 10:27:03 +0000 [thread overview] Message-ID: <7caf5127-36fc-7c77-00f1-7be82d6f26e0@csgroup.eu> (raw) In-Reply-To: <877dfrrkxo.fsf@disp2133> On 9/8/21 6:17 PM, Eric W. Biederman wrote: > Christophe Leroy <christophe.leroy@csgroup.eu> writes: > >> Le 02/09/2021 à 20:43, Eric W. Biederman a écrit : >>> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> >>>> In the same spirit as commit fb05121fd6a2 ("signal: Add >>>> unsafe_get_compat_sigset()"), implement an 'unsafe' version of >>>> copy_siginfo_to_user() in order to use it within user access blocks. >>>> >>>> For that, also add an 'unsafe' version of clear_user(). >>> >>> Looking at your use cases you need the 32bit compat version of this >>> as well. >>> >>> The 32bit compat version is too complicated to become a macro, so I >>> don't think you can make this work correctly for the 32bit compat case. >> >> When looking into patch 5/5 that you nacked, I think you missed the fact that we >> keep using copy_siginfo_to_user32() as it for the 32 bit compat case. > > I did. My mistake. > > However that mistake was so easy I think it mirrors the comments others > have made that this looks like a maintenance hazard. > > Is improving the performance of 32bit kernels interesting? Yes it is, and that's what this series do. > Is improving the performance of 32bit compat support interesting? For me this is a corner case, so I left it aside for now. > > If performance one or either of those cases is interesting it looks like > we already have copy_siginfo_to_external32 the factor you would need > to build unsafe_copy_siginfo_to_user32. I'm not sure I understand your saying here. What do you expect me to do with copy_siginfo_to_external32() ? copy_siginfo_to_user32() is for compat only. Native 32 bits powerpc use copy_siginfo_to_user() > > So I am not going to say impossible but please make something > maintainable. I unified all of the compat 32bit siginfo logic because > it simply did not get enough love and attention when it was implemented > per architecture. Yes, and ? I didn't do any modification to the compat case, so what you did remains. > > In general I think that concern applies to this case as well. We really > need an implementation that shares as much burden as possible with other > architectures. I think yes, that's the reason why I made a generic unsafe_copy_siginfo_to_user() and didn't make a powerpc dedicated change. Once this is merged any other architecture can use unsafe_copy_siginfo_to_user(). Did I miss something ? Christophe
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu> To: "Eric W. Biederman" <ebiederm@xmission.com> Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras <paulus@samba.org>, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/5] signal: Add unsafe_copy_siginfo_to_user() Date: Fri, 10 Sep 2021 10:27:03 +0000 [thread overview] Message-ID: <7caf5127-36fc-7c77-00f1-7be82d6f26e0@csgroup.eu> (raw) In-Reply-To: <877dfrrkxo.fsf@disp2133> On 9/8/21 6:17 PM, Eric W. Biederman wrote: > Christophe Leroy <christophe.leroy@csgroup.eu> writes: > >> Le 02/09/2021 à 20:43, Eric W. Biederman a écrit : >>> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> >>>> In the same spirit as commit fb05121fd6a2 ("signal: Add >>>> unsafe_get_compat_sigset()"), implement an 'unsafe' version of >>>> copy_siginfo_to_user() in order to use it within user access blocks. >>>> >>>> For that, also add an 'unsafe' version of clear_user(). >>> >>> Looking at your use cases you need the 32bit compat version of this >>> as well. >>> >>> The 32bit compat version is too complicated to become a macro, so I >>> don't think you can make this work correctly for the 32bit compat case. >> >> When looking into patch 5/5 that you nacked, I think you missed the fact that we >> keep using copy_siginfo_to_user32() as it for the 32 bit compat case. > > I did. My mistake. > > However that mistake was so easy I think it mirrors the comments others > have made that this looks like a maintenance hazard. > > Is improving the performance of 32bit kernels interesting? Yes it is, and that's what this series do. > Is improving the performance of 32bit compat support interesting? For me this is a corner case, so I left it aside for now. > > If performance one or either of those cases is interesting it looks like > we already have copy_siginfo_to_external32 the factor you would need > to build unsafe_copy_siginfo_to_user32. I'm not sure I understand your saying here. What do you expect me to do with copy_siginfo_to_external32() ? copy_siginfo_to_user32() is for compat only. Native 32 bits powerpc use copy_siginfo_to_user() > > So I am not going to say impossible but please make something > maintainable. I unified all of the compat 32bit siginfo logic because > it simply did not get enough love and attention when it was implemented > per architecture. Yes, and ? I didn't do any modification to the compat case, so what you did remains. > > In general I think that concern applies to this case as well. We really > need an implementation that shares as much burden as possible with other > architectures. I think yes, that's the reason why I made a generic unsafe_copy_siginfo_to_user() and didn't make a powerpc dedicated change. Once this is merged any other architecture can use unsafe_copy_siginfo_to_user(). Did I miss something ? Christophe
next prev parent reply other threads:[~2021-09-10 10:27 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-23 15:35 [PATCH v2 1/5] powerpc/signal64: Access function descriptor with user access block Christophe Leroy 2021-08-23 15:35 ` Christophe Leroy 2021-08-23 15:35 ` [PATCH v2 2/5] powerpc/signal: Include the new stack frame inside the " Christophe Leroy 2021-08-23 15:35 ` Christophe Leroy 2021-08-23 15:35 ` [PATCH v2 3/5] signal: Add unsafe_copy_siginfo_to_user() Christophe Leroy 2021-08-23 15:35 ` Christophe Leroy 2021-09-02 6:54 ` Christoph Hellwig 2021-09-02 6:54 ` Christoph Hellwig 2021-09-02 16:05 ` Linus Torvalds 2021-09-02 16:05 ` Linus Torvalds 2021-09-13 12:59 ` Christophe Leroy 2021-09-13 12:59 ` Christophe Leroy 2021-09-02 18:43 ` Eric W. Biederman 2021-09-02 18:43 ` Eric W. Biederman 2021-09-03 8:56 ` Christophe Leroy 2021-09-03 8:56 ` Christophe Leroy 2021-09-08 18:17 ` Eric W. Biederman 2021-09-08 18:17 ` Eric W. Biederman 2021-09-10 10:27 ` Christophe Leroy [this message] 2021-09-10 10:27 ` Christophe Leroy 2021-09-11 15:58 ` Eric W. Biederman 2021-09-11 15:58 ` Eric W. Biederman 2021-09-13 12:56 ` Christophe Leroy 2021-09-13 12:56 ` Christophe Leroy 2021-08-23 15:35 ` [PATCH v2 4/5] powerpc/uaccess: Add unsafe_clear_user() Christophe Leroy 2021-08-23 15:35 ` Christophe Leroy 2021-08-23 15:35 ` [PATCH v2 5/5] powerpc/signal: Use unsafe_copy_siginfo_to_user() Christophe Leroy 2021-08-23 15:35 ` Christophe Leroy 2021-09-02 18:38 ` Eric W. Biederman 2021-09-02 18:38 ` Eric W. Biederman 2021-09-03 8:53 ` Christophe Leroy 2021-09-03 8:53 ` Christophe Leroy 2021-09-02 6:49 ` [PATCH v2 1/5] powerpc/signal64: Access function descriptor with user access block Christoph Hellwig 2021-09-02 6:49 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=7caf5127-36fc-7c77-00f1-7be82d6f26e0@csgroup.eu \ --to=christophe.leroy@csgroup.eu \ --cc=benh@kernel.crashing.org \ --cc=ebiederm@xmission.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.