Re: [PATCH v3 12/12] signalfd: Remove SIL_FAULT_PERF_EVENT fields from signalfd_siginfo

[Marco Elver]

On Wed, 5 May 2021 at 16:11, Eric W. Beiderman <ebiederm@xxxxxxxxxxxx> wrote:
> From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>
> With the addition of ssi_perf_data and ssi_perf_type struct signalfd_siginfo
> is dangerously close to running out of space.  All that remains is just
> enough space for two additional 64bit fields.  A practice of adding all
> possible siginfo_t fields into struct singalfd_siginfo can not be supported
> as adding the missing fields ssi_lower, ssi_upper, and ssi_pkey would
> require two 64bit fields and one 32bit fields.  In practice the fields
> ssi_perf_data and ssi_perf_type can never be used by signalfd as the signal
> that generates them always delivers them synchronously to the thread that
> triggers them.
>
> Therefore until someone actually needs the fields ssi_perf_data and
> ssi_perf_type in signalfd_siginfo remove them.  This leaves a bit more room
> for future expansion.
>
> v1: https://lkml.kernel.org/r/20210503203814.25487-12-ebiederm@xxxxxxxxxxxx
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>

Reviewed-by: Marco Elver <elver@xxxxxxxxxx>


> ---
>  fs/signalfd.c                 | 16 ++++++----------
>  include/uapi/linux/signalfd.h |  4 +---
>  2 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/fs/signalfd.c b/fs/signalfd.c
> index 335ad39f3900..040e1cf90528 100644
> --- a/fs/signalfd.c
> +++ b/fs/signalfd.c
> @@ -114,12 +114,13 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
>                 break;
>         case SIL_FAULT_BNDERR:
>         case SIL_FAULT_PKUERR:
> +       case SIL_FAULT_PERF_EVENT:
>                 /*
> -                * Fall through to the SIL_FAULT case.  Both SIL_FAULT_BNDERR
> -                * and SIL_FAULT_PKUERR are only generated by faults that
> -                * deliver them synchronously to userspace.  In case someone
> -                * injects one of these signals and signalfd catches it treat
> -                * it as SIL_FAULT.
> +                * Fall through to the SIL_FAULT case.  SIL_FAULT_BNDERR,
> +                * SIL_FAULT_PKUERR, and SIL_FAULT_PERF_EVENT are only
> +                * generated by faults that deliver them synchronously to
> +                * userspace.  In case someone injects one of these signals
> +                * and signalfd catches it treat it as SIL_FAULT.
>                  */
>         case SIL_FAULT:
>                 new.ssi_addr = (long) kinfo->si_addr;
> @@ -132,11 +133,6 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
>                 new.ssi_addr = (long) kinfo->si_addr;
>                 new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
>                 break;
> -       case SIL_FAULT_PERF_EVENT:
> -               new.ssi_addr = (long) kinfo->si_addr;
> -               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;
>                 new.ssi_uid    = kinfo->si_uid;
> diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h
> index e78dddf433fc..83429a05b698 100644
> --- a/include/uapi/linux/signalfd.h
> +++ b/include/uapi/linux/signalfd.h
> @@ -39,8 +39,6 @@ struct signalfd_siginfo {
>         __s32 ssi_syscall;
>         __u64 ssi_call_addr;
>         __u32 ssi_arch;
> -       __u32 ssi_perf_type;
> -       __u64 ssi_perf_data;
>
>         /*
>          * Pad strcture to 128 bytes. Remember to update the
> @@ -51,7 +49,7 @@ struct signalfd_siginfo {
>          * comes out of a read(2) and we really don't want to have
>          * a compat on read(2).
>          */
> -       __u8 __pad[16];
> +       __u8 __pad[28];
>  };
>
>
> --
> 2.30.1