public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] parisc architecture fixes for v6.7-rc1
@ 2023-10-31 10:27 Helge Deller
  2023-11-01 20:50 ` pr-tracker-bot
  0 siblings, 1 reply; 7+ messages in thread
From: Helge Deller @ 2023-10-31 10:27 UTC (permalink / raw)
  To: Linus Torvalds, linux-kernel, linux-parisc, James Bottomley,
	John David Anglin

Hi Linus,

please pull the fixes and updates for the parisc architecture for 6.7-rc1:

Usual fixes and updates:
* Add up to 12 nops after TLB inserts for PA8x00 CPUs as the specification
  requires (Dave Anglin)
* Simplify the parisc smp_prepare_boot_cpu() code (Russell King)
* Use 64-bit little-endian values in SBA IOMMU PDIR table for AGP

Since there is upcoming support for booting a 64-bit kernel on QEMU,
some corner cases were fixed and improvements added:
* Fix 64-bit kernel crash in STI (graphics console) font setup code
* Support building an uncompressed Linux kernel
* Add support for soft power-off in qemu

Thanks,
Helge

----------------------------------------------------------------
The following changes since commit 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1:

  Linux 6.6-rc7 (2023-10-22 12:11:21 -1000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git tags/parisc-for-6.7-rc1

for you to fetch changes up to 8a32aa17c1cd48df1ddaa78e45abcb8c7a2220d6:

  fbdev: stifb: Make the STI next font pointer a 32-bit signed offset (2023-10-30 14:54:41 +0100)

----------------------------------------------------------------
parisc architecture fixes and updates for kernel v6.7-rc1:

- Add nop instructions after TLB inserts for PA8x00 CPUs
- Fix a 64-bit kernel crash in STI font routines which miscalculates
  the font start address as it gets signed vs unsigned offsets wrong
- Support building an uncompressed Linux kernel
- Simplify smp_prepare_boot_cpu() function
- Support for soft power-off in qemu
- Use 64-bit little-endian values in SBA IOMMU PDIR table for AGP

----------------------------------------------------------------
Helge Deller (11):
      parisc: sba-iommu: Fix comment when calculating IOC number
      parisc: Add some missing PDC functions and constants
      parisc: Allow building uncompressed Linux kernel
      parisc/power: Add power soft-off when running on qemu
      parisc/power: Trivial whitespace cleanups and license update
      parisc: Move parisc_narrow_firmware variable to header file
      parisc/firmware: Use PDC constants for narrow/wide firmware
      parisc/agp: Use 64-bit LE values in SBA IOMMU PDIR table
      parisc/pdc: Add width field to struct pdc_model
      parisc: Show default CPU PSW.W setting as reported by PDC
      fbdev: stifb: Make the STI next font pointer a 32-bit signed offset

John David Anglin (1):
      parisc: Add nop instructions after TLB inserts

Russell King (Oracle) (1):
      parisc: simplify smp_prepare_boot_cpu()

 arch/parisc/Kconfig                 |  1 +
 arch/parisc/boot/Makefile           |  2 +-
 arch/parisc/include/asm/processor.h |  1 +
 arch/parisc/include/uapi/asm/pdc.h  | 28 ++++++++++++-
 arch/parisc/kernel/drivers.c        |  4 +-
 arch/parisc/kernel/entry.S          | 81 ++++++++++++++++++++++++-------------
 arch/parisc/kernel/firmware.c       | 14 +++----
 arch/parisc/kernel/processor.c      |  6 +--
 arch/parisc/kernel/setup.c          |  3 --
 arch/parisc/kernel/smp.c            |  8 +---
 drivers/char/agp/parisc-agp.c       | 16 ++++----
 drivers/parisc/power.c              | 67 +++++++++++++-----------------
 drivers/parisc/sba_iommu.c          |  2 +-
 include/video/sticore.h             |  2 +-
 14 files changed, 133 insertions(+), 102 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] parisc architecture fixes for v6.7-rc1
  2023-10-31 10:27 [GIT PULL] parisc architecture fixes for v6.7-rc1 Helge Deller
@ 2023-11-01 20:50 ` pr-tracker-bot
  0 siblings, 0 replies; 7+ messages in thread
From: pr-tracker-bot @ 2023-11-01 20:50 UTC (permalink / raw)
  To: Helge Deller
  Cc: Linus Torvalds, linux-kernel, linux-parisc, James Bottomley,
	John David Anglin

The pull request you sent on Tue, 31 Oct 2023 11:27:18 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git tags/parisc-for-6.7-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f00593e09968ed6dfcd10aebb13f470fbe3343b4

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [GIT PULL] parisc architecture fixes for v6.7-rc1
@ 2023-11-18 13:58 Helge Deller
  2023-11-18 17:36 ` Linus Torvalds
  0 siblings, 1 reply; 7+ messages in thread
From: Helge Deller @ 2023-11-18 13:58 UTC (permalink / raw)
  To: Linus Torvalds, linux-kernel, linux-parisc, James Bottomley,
	John David Anglin
  Cc: Kees Cook

Hi Linus,

please pull three small additional parisc fixes for kernel 6.7-rc2,
two of those are tagged for stable series.

On parisc we still sometimes need writeable stacks, e.g. if programs aren't
compiled with gcc-14. To avoid issues with the upcoming systemd-254 we
therefore have to disable prctl(PR_SET_MDWE) for now (for parisc only).

The other two patches are minor: a bugfix for the soft power-off on qemu
with 64-bit kernel and a patch from Kees to prefer strscpy() over strlcpy().

Thanks!
Helge

----------------------------------------------------------------
The following changes since commit b85ea95d086471afb4ad062012a4d73cd328fa86:

  Linux 6.7-rc1 (2023-11-12 16:19:07 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git tags/parisc-for-6.7-rc2

for you to fetch changes up to b8eaae484f79b37c602d112e131475013ab14519:

  parisc/power: Fix power soft-off when running on qemu (2023-11-17 16:54:27 +0100)

----------------------------------------------------------------
parisc architecture fixes for kernel v6.7-rc2:

- Fix power soft-off on qemu
- Disable prctl(PR_SET_MDWE) since parisc sometimes still needs
  writeable stacks
- Use strscpy instead of strlcpy in show_cpuinfo()

----------------------------------------------------------------
Helge Deller (2):
      prctl: Temporarily disable prctl(PR_SET_MDWE) on parisc
      parisc/power: Fix power soft-off when running on qemu

Kees Cook (1):
      parisc: Replace strlcpy() with strscpy()

 arch/parisc/kernel/processor.c |  2 +-
 drivers/parisc/power.c         |  2 +-
 kernel/sys.c                   | 10 ++++++++--
 3 files changed, 10 insertions(+), 4 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] parisc architecture fixes for v6.7-rc1
  2023-11-18 13:58 Helge Deller
@ 2023-11-18 17:36 ` Linus Torvalds
  2023-11-18 18:40   ` Helge Deller
  0 siblings, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2023-11-18 17:36 UTC (permalink / raw)
  To: Helge Deller
  Cc: linux-kernel, linux-parisc, James Bottomley, John David Anglin,
	Kees Cook

On Sat, 18 Nov 2023 at 05:58, Helge Deller <deller@gmx.de> wrote:
>
> On parisc we still sometimes need writeable stacks, e.g. if programs aren't
> compiled with gcc-14. To avoid issues with the upcoming systemd-254 we
> therefore have to disable prctl(PR_SET_MDWE) for now (for parisc only).

Ugh.

I pulled this, but I *really* cannot live with how ugly that is.

Seriously, that code is just unacceptable. Doing something like

+               if (IS_ENABLED(CONFIG_PARISC))
+                       error = -EINVAL;
+               else
+                       error = prctl_set_mdwe(arg2, arg3, arg4, arg5);

in generic code with no comment is just truly crazy. If you have to go
and do a "git blame -C" just to understand why the code exists, the
code is a problem.

But it goes beyond that. The code is just *ugly*, and it's done
entirely in the wrong place.

Things like "mdwe is special on parisc" should *NOT* be done in the
generic "prctl()" function.  This issue is not specific to prctl() -
it's very much specific to mdwe.

So I think it would have been both much more legible, and *much* more
appropriate, to do it in prctl_set_mdwe() itself, where it makes more
sense, and where it matches all the *other* mdwe-specific checks the
code does wrt arguments and existing state.

And honestly, why wouldn't 'get_mdwe' work? So the *other* hunk in
that patch (which isn't even mentioned in the commit message) that
returns -EINVAL for get_mdwe makes no sense at all, and shouldn't have
existed.

End result: I think the code should have been something like this
(whitespace-damaged) thing:

  --- a/kernel/sys.c
  +++ b/kernel/sys.c
  @@ -2394,6 +2394,10 @@ static inline int prctl_set_mdwe(unsigned
long bits,
       if (bits & PR_MDWE_NO_INHERIT && !(bits & PR_MDWE_REFUSE_EXEC_GAIN))
           return -EINVAL;

  +    /* PARISC cannot allow mdwe as it needs writable stacks */
  +    if (IS_ENABLED(CONFIG_PARISC))
  +        return  -ENOSYS;
  +
       current_bits = get_current_mdwe();
       if (current_bits && current_bits != bits)
           return -EPERM; /* Cannot unset the flags */

where I also picked another error code, because it's not that the
prctl value or the arguments are invalid, I think the error should
show that there's something else going on.

No, I don't think -ENOSYS is necessarily the best possible error
value, but I think it at least conceptually matches the "this prctl
doesn't exist on PARISC". Maybe

Maybe ENOSYS should be avoided (prctl() obvious does exist), but I do
think this should be a different error than the EINVAL that the
generic checks do.

End result: I really hated this change so much that I ended up
unpulling after doing the pull. This needs to be done right, or not at
all.

              Linus

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] parisc architecture fixes for v6.7-rc1
  2023-11-18 17:36 ` Linus Torvalds
@ 2023-11-18 18:40   ` Helge Deller
  2023-11-18 18:42     ` Linus Torvalds
  0 siblings, 1 reply; 7+ messages in thread
From: Helge Deller @ 2023-11-18 18:40 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, linux-parisc, James Bottomley, John David Anglin,
	Kees Cook, Sam James

On 11/18/23 18:36, Linus Torvalds wrote:
> On Sat, 18 Nov 2023 at 05:58, Helge Deller <deller@gmx.de> wrote:
>>
>> On parisc we still sometimes need writeable stacks, e.g. if programs aren't
>> compiled with gcc-14. To avoid issues with the upcoming systemd-254 we
>> therefore have to disable prctl(PR_SET_MDWE) for now (for parisc only).
>
> Ugh.
>
> I pulled this, but I *really* cannot live with how ugly that is.
>
> Seriously, that code is just unacceptable. Doing something like
>
> +               if (IS_ENABLED(CONFIG_PARISC))
> +                       error = -EINVAL;
> +               else
> +                       error = prctl_set_mdwe(arg2, arg3, arg4, arg5);
>
> in generic code with no comment is just truly crazy. If you have to go
> and do a "git blame -C" just to understand why the code exists, the
> code is a problem.
>
> But it goes beyond that. The code is just *ugly*, and it's done
> entirely in the wrong place.
>
> Things like "mdwe is special on parisc" should *NOT* be done in the
> generic "prctl()" function.  This issue is not specific to prctl() -
> it's very much specific to mdwe.
>
> So I think it would have been both much more legible, and *much* more
> appropriate, to do it in prctl_set_mdwe() itself, where it makes more
> sense, and where it matches all the *other* mdwe-specific checks the
> code does wrt arguments and existing state.
>
> And honestly, why wouldn't 'get_mdwe' work? So the *other* hunk in
> that patch (which isn't even mentioned in the commit message) that
> returns -EINVAL for get_mdwe makes no sense at all, and shouldn't have
> existed.
>
> End result: I think the code should have been something like this
> (whitespace-damaged) thing:
>
>    --- a/kernel/sys.c
>    +++ b/kernel/sys.c
>    @@ -2394,6 +2394,10 @@ static inline int prctl_set_mdwe(unsigned
> long bits,
>         if (bits & PR_MDWE_NO_INHERIT && !(bits & PR_MDWE_REFUSE_EXEC_GAIN))
>             return -EINVAL;
>
>    +    /* PARISC cannot allow mdwe as it needs writable stacks */
>    +    if (IS_ENABLED(CONFIG_PARISC))
>    +        return  -ENOSYS;
>    +
>         current_bits = get_current_mdwe();
>         if (current_bits && current_bits != bits)
>             return -EPERM; /* Cannot unset the flags */

Ok.
My initial patch was actually doing exatly that, but somehow I finally decided
to add it to the switch() instead. Seems this was the wrong decision :-(

> where I also picked another error code, because it's not that the
> prctl value or the arguments are invalid, I think the error should
> show that there's something else going on.
>
> No, I don't think -ENOSYS is necessarily the best possible error
> value, but I think it at least conceptually matches the "this prctl
> doesn't exist on PARISC". Maybe
>
> Maybe ENOSYS should be avoided (prctl() obvious does exist), but I do
> think this should be a different error than the EINVAL that the
> generic checks do.

I agree that returning something else than EINVAL would be better.
I used ENODEV in an earlier patch (I didn't liked it either), but according to
https://github.com/systemd/systemd/issues/29775#issuecomment-1809563365
EINVAL seems the best solution currently.

Just as a side-note: ENOSYS gives a checkpatch warning:
WARNING: ENOSYS means 'invalid syscall nr' and nothing else

Would the patch below be OK? It's basically yours but with EINVAL.
(might be whitespace-scrambled!)

Helge

---

From: Helge Deller <deller@gmx.de>
Subject: [PATCH] prctl: Disable prctl(PR_SET_MDWE) on parisc

systemd-254 tries to use prctl(PR_SET_MDWE) for it's MemoryDenyWriteExecute
functionality, but fails on parisc which still needs executable stacks in
certain combinations of gcc/glibc/kernel.

Disable prctl(PR_SET_MDWE) by returning -EINVAL for now on parisc, until
userspace has catched up.

Signed-off-by: Helge Deller <deller@gmx.de>
Co-developed-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Sam James <sam@gentoo.org>
Closes: https://github.com/systemd/systemd/issues/29775
Tested-by: Sam James <sam@gentoo.org>
Link: https://lore.kernel.org/all/875y2jro9a.fsf@gentoo.org/
Cc: <stable@vger.kernel.org> # v6.3+

diff --git a/kernel/sys.c b/kernel/sys.c
index 420d9cb9cc8e..e219fcfa112d 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2394,6 +2394,10 @@ static inline int prctl_set_mdwe(unsigned long bits, unsigned long arg3,
         if (bits & PR_MDWE_NO_INHERIT && !(bits & PR_MDWE_REFUSE_EXEC_GAIN))
                 return -EINVAL;

+       /* PARISC cannot allow mdwe as it needs writable stacks */
+       if (IS_ENABLED(CONFIG_PARISC))
+               return -EINVAL;
+
         current_bits = get_current_mdwe();
         if (current_bits && current_bits != bits)
                 return -EPERM; /* Cannot unset the flags */

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] parisc architecture fixes for v6.7-rc1
  2023-11-18 18:40   ` Helge Deller
@ 2023-11-18 18:42     ` Linus Torvalds
  2023-11-18 18:47       ` Helge Deller
  0 siblings, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2023-11-18 18:42 UTC (permalink / raw)
  To: Helge Deller
  Cc: linux-kernel, linux-parisc, James Bottomley, John David Anglin,
	Kees Cook, Sam James

On Sat, 18 Nov 2023 at 10:40, Helge Deller <deller@gmx.de> wrote:
>
> Would the patch below be OK? It's basically yours but with EINVAL.
> (might be whitespace-scrambled!)

I don't particularly like EINVAL, but it's not the kind of
show-stopper that the other issues were.

               Linus

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] parisc architecture fixes for v6.7-rc1
  2023-11-18 18:42     ` Linus Torvalds
@ 2023-11-18 18:47       ` Helge Deller
  0 siblings, 0 replies; 7+ messages in thread
From: Helge Deller @ 2023-11-18 18:47 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, linux-parisc, James Bottomley, John David Anglin,
	Kees Cook, Sam James

On 11/18/23 19:42, Linus Torvalds wrote:
> On Sat, 18 Nov 2023 at 10:40, Helge Deller <deller@gmx.de> wrote:
>>
>> Would the patch below be OK? It's basically yours but with EINVAL.
>> (might be whitespace-scrambled!)
>
> I don't particularly like EINVAL, but it's not the kind of
> show-stopper that the other issues were.

Ok, thank you!
I'll send you a new pull request.

Btw, we are in the process to get rid of executable stacks, but
this will take time.
That said, I hope to remove this check then again.

Helge

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-11-18 18:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-31 10:27 [GIT PULL] parisc architecture fixes for v6.7-rc1 Helge Deller
2023-11-01 20:50 ` pr-tracker-bot
  -- strict thread matches above, loose matches on Subject: below --
2023-11-18 13:58 Helge Deller
2023-11-18 17:36 ` Linus Torvalds
2023-11-18 18:40   ` Helge Deller
2023-11-18 18:42     ` Linus Torvalds
2023-11-18 18:47       ` Helge Deller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox