From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2B8FC43460 for ; Sun, 2 May 2021 18:39:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A38FF611F1 for ; Sun, 2 May 2021 18:39:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230036AbhEBSkQ (ORCPT ); Sun, 2 May 2021 14:40:16 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:44752 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229686AbhEBSkQ (ORCPT ); Sun, 2 May 2021 14:40:16 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out02.mta.xmission.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1ldGzy-00FCZQ-V6; Sun, 02 May 2021 12:39:23 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1ldGzx-0006hK-2a; Sun, 02 May 2021 12:39:22 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Marco Elver Cc: Arnd Bergmann , Florian Weimer , "David S. Miller" , Peter Zijlstra , Ingo Molnar , Thomas Gleixner , Peter Collingbourne , Dmitry Vyukov , Alexander Potapenko , sparclinux , linux-arch , Linux Kernel Mailing List , Linux API , kasan-dev References: Date: Sun, 02 May 2021 13:39:16 -0500 In-Reply-To: (Marco Elver's message of "Sat, 1 May 2021 12:47:21 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1ldGzx-0006hK-2a;;;mid=;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/61Mdwb7TuChJAy6IEn9kCmxkmOP4SzJw= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH 7/3] signal: Deliver all of the perf_data in si_perf X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Precedence: bulk List-ID: X-Mailing-List: linux-api@vger.kernel.org Marco Elver writes: > On Sat, 1 May 2021 at 01:44, Eric W. Biederman wrote: >> >> Don't abuse si_errno and deliver all of the perf data in si_perf. >> >> Signed-off-by: "Eric W. Biederman" >> --- > > Thank you for the fix, this looks cleaner. > > Just note that this patch needs to include updates to > tools/testing/selftests/perf_events. This should do it: >> sed -i 's/si_perf/si_perf.data/g; s/si_errno/si_perf.type/g' tools/testing/selftests/perf_events/*.c > > Subject: s/perf_data/perf data/ ? > > For uapi, need to switch to __u32, see below. Good point. The one thing that this doesn't do is give you a 64bit field on 32bit architectures. On 32bit builds the layout is: int si_signo; int si_errno; int si_code; void __user *_addr; So I believe if the first 3 fields were moved into the _sifields union si_perf could define a 64bit field as it's first member and it would not break anything else. Given that the data field is 64bit that seems desirable. Eric >> fs/signalfd.c | 3 ++- >> include/linux/compat.h | 5 ++++- >> include/uapi/asm-generic/siginfo.h | 5 ++++- >> include/uapi/linux/signalfd.h | 4 ++-- >> kernel/signal.c | 18 +++++++++++------- >> 5 files changed, 23 insertions(+), 12 deletions(-) >> >> diff --git a/fs/signalfd.c b/fs/signalfd.c >> index 83130244f653..9686af56f073 100644 >> --- a/fs/signalfd.c >> +++ b/fs/signalfd.c >> @@ -134,7 +134,8 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo, >> break; >> case SIL_FAULT_PERF_EVENT: >> new.ssi_addr = (long) kinfo->si_addr; >> - new.ssi_perf = kinfo->si_perf; >> + new.ssi_perf_type = kinfo->si_perf.type; >> + new.ssi_perf_data = kinfo->si_perf.data; >> break; >> case SIL_CHLD: >> new.ssi_pid = kinfo->si_pid; >> diff --git a/include/linux/compat.h b/include/linux/compat.h >> index 24462ed63af4..0726f9b3a57c 100644 >> --- a/include/linux/compat.h >> +++ b/include/linux/compat.h >> @@ -235,7 +235,10 @@ typedef struct compat_siginfo { >> u32 _pkey; >> } _addr_pkey; >> /* used when si_code=TRAP_PERF */ >> - compat_ulong_t _perf; >> + struct { >> + compat_ulong_t data; >> + u32 type; >> + } _perf; >> }; >> } _sigfault; >> >> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h >> index 2abdf1d19aad..19b6310021a3 100644 >> --- a/include/uapi/asm-generic/siginfo.h >> +++ b/include/uapi/asm-generic/siginfo.h >> @@ -90,7 +90,10 @@ union __sifields { >> __u32 _pkey; >> } _addr_pkey; >> /* used when si_code=TRAP_PERF */ >> - unsigned long _perf; >> + struct { >> + unsigned long data; >> + u32 type; > > This needs to be __u32. > > >> + } _perf; >> }; >> } _sigfault; >> >> diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h >> index 7e333042c7e3..e78dddf433fc 100644 >> --- a/include/uapi/linux/signalfd.h >> +++ b/include/uapi/linux/signalfd.h >> @@ -39,8 +39,8 @@ struct signalfd_siginfo { >> __s32 ssi_syscall; >> __u64 ssi_call_addr; >> __u32 ssi_arch; >> - __u32 __pad3; >> - __u64 ssi_perf; >> + __u32 ssi_perf_type; >> + __u64 ssi_perf_data; >> >> /* >> * Pad strcture to 128 bytes. Remember to update the >> diff --git a/kernel/signal.c b/kernel/signal.c >> index 5b1ad7f080ab..cb3574b7319c 100644 >> --- a/kernel/signal.c >> +++ b/kernel/signal.c >> @@ -1758,11 +1758,13 @@ int force_sig_perf(void __user *pending_addr, u32 type, u64 sig_data) >> struct kernel_siginfo info; >> >> clear_siginfo(&info); >> - info.si_signo = SIGTRAP; >> - info.si_errno = type; >> - info.si_code = TRAP_PERF; >> - info.si_addr = pending_addr; >> - info.si_perf = sig_data; >> + info.si_signo = SIGTRAP; >> + info.si_errno = 0; >> + info.si_code = TRAP_PERF; >> + info.si_addr = pending_addr; >> + info.si_perf.data = sig_data; >> + info.si_perf.type = type; >> + >> return force_sig_info(&info); >> } >> >> @@ -3379,7 +3381,8 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, >> break; >> case SIL_FAULT_PERF_EVENT: >> to->si_addr = ptr_to_compat(from->si_addr); >> - to->si_perf = from->si_perf; >> + to->si_perf.data = from->si_perf.data; >> + to->si_perf.type = from->si_perf.type; >> break; >> case SIL_CHLD: >> to->si_pid = from->si_pid; >> @@ -3455,7 +3458,8 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, >> break; >> case SIL_FAULT_PERF_EVENT: >> to->si_addr = compat_ptr(from->si_addr); >> - to->si_perf = from->si_perf; >> + to->si_perf.data = from->si_perf.data; >> + to->si_perf.type = from->si_perf.type; >> break; >> case SIL_CHLD: >> to->si_pid = from->si_pid; >> -- >> 2.30.1