Re: [PATCH 0/6] Final si_trapno bits

Marco Elver [off-list ref] writes:

On Thu, Jul 15, 2021 at 01:09PM -0500, Eric W. Biederman wrote:

quoted

As a part of a fix for the ABI of the newly added SIGTRAP TRAP_PERF a
si_trapno was reduced to an ordinary extention of the _sigfault case
of struct siginfo.

When Linus saw the complete set of changes come in as a fix he requested
that the set of changes be trimmed down to just what was necessary to
fix the SIGTRAP TRAP_PERF ABI.

I had intended to get the rest of the changes into the merge window for
v5.14 but I dropped the ball.

I have made the changes to stop using __ARCH_SI_TRAPNO be per
architecture so they are easier to review.  In doing so I found one
place on alpha where I used send_sig_fault instead of
send_sig_fault_trapno(... si_trapno = 0).  That would not have changed
the userspace behavior but it did make the kernel code less clear.

My rule in these patches is everywhere that siginfo layout calls
for SIL_FAULT_TRAPNO the code uses either force_sig_fault_trapno
or send_sig_fault_trapno.

And of course I have rebased and compile tested Marco's compile time
assert patches.

Eric


Eric W. Biederman (3):
      signal/sparc: si_trapno is only used with SIGILL ILL_ILLTRP
      signal/alpha: si_trapno is only used with SIGFPE and SIGTRAP TRAP_UNK
      signal: Remove the generic __ARCH_SI_TRAPNO support

Marco Elver (3):
      sparc64: Add compile-time asserts for siginfo_t offsets
      arm: Add compile-time asserts for siginfo_t offsets
      arm64: Add compile-time asserts for siginfo_t offsets

Nice, thanks for the respin. If I diffed it right, I see this is almost
(modulo what you mentioned above) equivalent to:
  https://lore.kernel.org/linux-api/m1tuni8ano.fsf_-_@fess.ebiederm.org/
+ what's already in mainline. It's only missing:

	signal: Verify the alignment and size of siginfo_t
	signal: Rename SIL_PERF_EVENT SIL_FAULT_PERF_EVENT for consistency

Would this be appropriate for this series, or rather separately, or
dropped completely?

Appropriate I just overlooked them.

Eric

On Fri, 16 Jul 2021 at 18:09, Eric W. Biederman [off-list ref] wrote:

Marco Elver [off-list ref] writes:

quoted

On Thu, Jul 15, 2021 at 01:09PM -0500, Eric W. Biederman wrote:

quoted

As a part of a fix for the ABI of the newly added SIGTRAP TRAP_PERF a
si_trapno was reduced to an ordinary extention of the _sigfault case
of struct siginfo.

When Linus saw the complete set of changes come in as a fix he requested
that the set of changes be trimmed down to just what was necessary to
fix the SIGTRAP TRAP_PERF ABI.

I had intended to get the rest of the changes into the merge window for
v5.14 but I dropped the ball.

I have made the changes to stop using __ARCH_SI_TRAPNO be per
architecture so they are easier to review.  In doing so I found one
place on alpha where I used send_sig_fault instead of
send_sig_fault_trapno(... si_trapno = 0).  That would not have changed
the userspace behavior but it did make the kernel code less clear.

My rule in these patches is everywhere that siginfo layout calls
for SIL_FAULT_TRAPNO the code uses either force_sig_fault_trapno
or send_sig_fault_trapno.

And of course I have rebased and compile tested Marco's compile time
assert patches.

Eric


Eric W. Biederman (3):
      signal/sparc: si_trapno is only used with SIGILL ILL_ILLTRP
      signal/alpha: si_trapno is only used with SIGFPE and SIGTRAP TRAP_UNK
      signal: Remove the generic __ARCH_SI_TRAPNO support

Marco Elver (3):
      sparc64: Add compile-time asserts for siginfo_t offsets
      arm: Add compile-time asserts for siginfo_t offsets
      arm64: Add compile-time asserts for siginfo_t offsets

Nice, thanks for the respin. If I diffed it right, I see this is almost
(modulo what you mentioned above) equivalent to:
  https://lore.kernel.org/linux-api/m1tuni8ano.fsf_-_@fess.ebiederm.org/
+ what's already in mainline. It's only missing:

      signal: Verify the alignment and size of siginfo_t
      signal: Rename SIL_PERF_EVENT SIL_FAULT_PERF_EVENT for consistency

Would this be appropriate for this series, or rather separately, or
dropped completely?

Appropriate I just overlooked them.

Full series with the 2 patches just sent looks good to me.

Thanks,
-- Marco