NetBSD Problem Report #52675

From martin@duskware.de  Mon Oct 30 12:13:56 2017
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id D71E57A1CA
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 30 Oct 2017 12:13:56 +0000 (UTC)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: crunching binaries improperly removes PaX notes
X-Send-Pr-Version: 3.95

>Number:         52675
>Category:       toolchain
>Synopsis:       crunching binaries improperly removes PaX notes
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    toolchain-manager
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Oct 30 12:15:00 +0000 2017
>Closed-Date:    Fri Jul 07 07:58:18 +0000 2023
>Last-Modified:  Fri Jul 07 07:58:18 +0000 2023
>Originator:     Martin Husemann
>Release:        NetBSD 8.99.5
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD night-owl.duskware.de 8.99.5 NetBSD 8.99.5 (NIGHT-OWL) #545: Mon Oct 23 13:44:09 CEST 2017 martin@night-owl.duskware.de:/usr/src/sys/arch/amd64/compile/NIGHT-OWL amd64
Architecture: x86_64
Machine: amd64
>Description:

When crunching multiple binaries into a single exe, the PaX notes are
cleared (as multiple of them would not make sense).

However, they are cleared,  not properly removed.

Compare readelf -n output for a statically build sbin/sysctl vs the
crunched rescue/sysctl:

static:

Displaying notes found at file offset 0x000000b4 with length 0x00000018:
  Owner                 Data size       Description
  NetBSD                0x00000004      IDENT 899000400 (8.99.4)

Displaying notes found at file offset 0x000000cc with length 0x00000014:
  Owner                 Data size       Description
  NetBSD                0x00000004      PaX <>

Displaying notes found at file offset 0x000000e0 with length 0x00000018:
  Owner                 Data size       Description
  NetBSD                0x00000004      MARCH <arm>

crunched:

Displaying notes found at file offset 0x000000b4 with length 0x00000018:
  Owner                 Data size       Description
  NetBSD                0x00000004      IDENT 899000400 (8.99.4)

Displaying notes found at file offset 0x000000e0 with length 0x00000018:
  Owner                 Data size       Description
  NetBSD                0x00000004      MARCH <arm>


The space in between has been zeroed, but this breaks note parsing
semantics (i.e. in arm vs. earm binaries getting their machine_arch
wrong).

>How-To-Repeat:

s/a

>Fix:
n/a

>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/52675: crunching binaries improperly removes PaX notes
Date: Thu, 30 Nov 2017 11:14:45 +0100

 In the hope of this being a simple binutils bug, I tried to reproduce it
 with objcopy -R - but that does the correct thing for a simple test case.

 On the other hand, crunchgen does not invoke paxctl or anything, just
 a slightly more complex version of objcopy.

 My test was the attached asm file, and then:

 cc test.s
 objcopy -R .note.demo.second a.out t.out
 readelf -a a.out
 readelf -a t.out

 ... and compare the file offsets for the "demo" notes.

 Martin

 # demo for objcopy bug, should work on all architectures (no real asm
 # in here)

 	.section .note.demo.first,"MG",@note,3*4+8+8,comdat
 	.p2align 2

 	.long	8
 	.long	8
 	.long	42
 	.ascii	"demo\0\0\0\0"
 	.ascii	"test234\0"
 	.p2align 2

 	.section .note.demo.second,"MG",@note,3*4+8+12,comdat
 	.p2align 2

 	.long	8
 	.long	12
 	.long	43
 	.ascii	"demo\0\0\0\0"
 	.ascii	"toberemoved\0"
 	.p2align 2

 	.section .note.demo.third,"MG",@note,3*4+8+8,comdat
 	.p2align 2

 	.long	8
 	.long	8
 	.long	44
 	.ascii	"demo\0\0\0\0"
 	.ascii	"the end\0"
 	.p2align 2

 .text
 	.global main
 main:
 	.long 0

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/52675: crunching binaries improperly removes PaX notes
Date: Mon, 11 Feb 2019 03:24:24 +0300

 On Mon, Oct 30, 2017 at 12:15:00 +0000, martin@NetBSD.org wrote:

 > The space in between has been zeroed, but this breaks note parsing
 > semantics (i.e. in arm vs. earm binaries getting their machine_arch
 > wrong).

 I wonder if instead of fixing objcopy we can do what we want of it
 manually with something like the following:

 Manually create a stub ELF note with namesz 0 that has the same total
 size as the PaX note and then use --update-section to replace the PaX
 note with the stub.


 fake-note.bin:
 .if ${TARGET_ENDIANNESS} == "1234"
 	printf '\0\0\0\0''\010\0\0\0''\0\0\0\0''\0\0\0\0''\0\0\0\0' > $@
 .else
 	printf '\0\0\0\0''\0\0\0\010''\0\0\0\0''\0\0\0\0''\0\0\0\0' > $@
 .endif

 ${PROG}.strip:
 	...
 	objcopy ... --update-section .note.netbsd.pax=fake-note.bin ...


 I get the following from readelf for the output of objcopy for a
 simple test binary:

 Displaying notes found at file offset 0x0000012c with length 0x00000018:
   Owner                 Data size       Description
   NetBSD                0x00000004      IDENT 899001800 (8.99.18)

 Displaying notes found at file offset 0x00000144 with length 0x00000014:
   Owner                 Data size       Description
   (NONE)               0x00000008       Unknown note type: (0x00000000)

 Displaying notes found at file offset 0x00000158 with length 0x00000020:
   Owner                 Data size       Description
   NetBSD                0x00000009      MARCH <earmv7hf>


 Verbose DIAGNOSTIC code still complains about the stub (it complains
 about ~everything), but doesn't get out of sync now and reads the
 MARCH note fine it seems:

   ./x.out: Unknown elf note type 0 (unknown tag): [namesz=0, descsz=8 name=]


 The note with namesz=0 and namesz=1 and name="" are reserved by the
 standard.

 If that's a problem we may instead just use a different name/type, we
 only need to make sure the size is the same and that our elf loader
 ignores that name/type.  E.g.

    0: namesz = 7
    4: descsz = 0
    8:   type = 3    # not currently in use
    c:   name = NetB
   10:          SD\0\0
   14:

 The section (well, the section header) ".note.netbsd.pax" is still
 there, but it shouldn't affect anything.  We can also tweak the name
 with e.g. --rename-section .note.netbsd.pax=.note.netbsd.xxx

 -uwe

From: Christos Zoulas <christos@zoulas.com>
To: "gnats-bugs@netbsd.org" <gnats-bugs@NetBSD.org>
Cc: toolchain-manager@netbsd.org,
 gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 "martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: toolchain/52675: crunching binaries improperly removes PaX notes
Date: Sun, 10 Feb 2019 21:05:30 -0500

 I think that we should just stop trying to remove the notes. The mcmodel =
 note is just used on sparc64 to enable topdown vm, and the pax note is =
 always 0 on the standard rescue built binaries. If one wants to fix new =
 crunched binaries that were built with inconsistent pax note settings, =
 one call paxctl(1) to adjust the the pax note settings. One we create =
 another tool to adjust mcmodel. Removing the notes makes the situation =
 strictly worse because objcopy(1) zeros out the note sections completely =
 instead of actually moving the notes around so there are no gaps. The =
 kernel loader reads the program header notes section and loses track =
 where it is because it can't calculate the size of the zeroed notes. Of =
 course the kernel can be modified to scan the section headers instead, =
 or we can inject fake contents to the zeroed out sections, but isn't it =
 better to leave them alone in the first place, and provide tools to =
 modify them (we have one for pax, we can add one for mcmodel if that's =
 necessary).

 christos=

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: Christos Zoulas <christos@zoulas.com>
Subject: Re: toolchain/52675: crunching binaries improperly removes PaX notes
Date: Mon, 11 Feb 2019 05:33:35 +0300

 On Sun, Feb 10, 2019 at 21:05:30 -0500, Christos Zoulas wrote:

 > The kernel loader reads the program header notes section and loses
 > track where it is because it can't calculate the size of the zeroed
 > notes.  Of course the kernel can be modified to scan the section
 > headers instead

 That would be wrong.  I fixed that couple of years ago.  Sections are
 for linkers, loaders should only ever use segments.

 -uwe

From: Christos Zoulas <christos@zoulas.com>
To: "gnats-bugs@netbsd.org" <gnats-bugs@NetBSD.org>
Cc: toolchain-manager@netbsd.org,
 gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 "martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: toolchain/52675: crunching binaries improperly removes PaX notes
Date: Sun, 10 Feb 2019 21:52:34 -0500

 Indeed, and I know the history... So the only viable solution for now is =
 to stop removing the notes.

 christos

 > On Feb 10, 2019, at 9:35 PM, Valery Ushakov <uwe@stderr.spb.ru> wrote:
 >=20
 > That would be wrong.  I fixed that couple of years ago.  Sections are
 > for linkers, loaders should only ever use segments.
 >=20
 > -uwe
 >=20

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>
Cc: netbsd-bugs@netbsd.org
Subject: Re: toolchain/52675 (crunching binaries improperly removes PaX notes)
Date: Mon, 5 Jun 2023 17:41:55 +0900

 --000000000000a3afca05fd5de0c7
 Content-Type: text/plain; charset="UTF-8"

 I've prepared a candidate fix for this PR:

 https://www.netbsd.org/~rin/pr52675.patch

 For crunchgen(1), clear PaX flags instead of removing its section.
 Otherwise, a zero-filled hole appears in ELF note segment for arm,
 for which MARCH note is located after PaX one.

 For this purpose, introduce -0 option for paxctl(8) to zero-clear
 PaX flags. IMO, this should be clearer than disabling all available
 flags.

 Comments?

 Thanks,
 rin

 --000000000000a3afca05fd5de0c7
 Content-Type: text/html; charset="UTF-8"
 Content-Transfer-Encoding: quoted-printable

 <div dir=3D"ltr">I&#39;ve prepared a candidate fix for this PR:<br><br><a h=
 ref=3D"https://www.netbsd.org/~rin/pr52675.patch">https://www.netbsd.org/~r=
 in/pr52675.patch</a><br><br>For crunchgen(1), clear PaX flags instead of re=
 moving its section.<br>Otherwise, a zero-filled hole appears in ELF note se=
 gment for arm,<br>for which MARCH note is located after PaX one.<br><br>For=
  this purpose, introduce -0 option for paxctl(8) to zero-clear<br>PaX flags=
 . IMO, this should be clearer than disabling all available<br>flags.<br><br=
 >Comments?<br><br>Thanks,<br>rin<br></div>

 --000000000000a3afca05fd5de0c7--

From: Martin Husemann <martin@duskware.de>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>
Subject: Re: toolchain/52675 (crunching binaries improperly removes PaX notes)
Date: Mon, 5 Jun 2023 11:16:10 +0200

 On Mon, Jun 05, 2023 at 05:41:55PM +0900, Rin Okuyama wrote:
 > For crunchgen(1), clear PaX flags instead of removing its section.
 > Otherwise, a zero-filled hole appears in ELF note segment for arm,
 > for which MARCH note is located after PaX one.

 I still wonder if this should be considered a binutils bug (worth
 reporting upstream?)

 > For this purpose, introduce -0 option for paxctl(8) to zero-clear
 > PaX flags. IMO, this should be clearer than disabling all available
 > flags.

 I like this idea and the patch looks good to me.

 Martin

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: Martin Husemann <martin@duskware.de>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>, netbsd-bugs@netbsd.org
Subject: Re: toolchain/52675 (crunching binaries improperly removes PaX notes)
Date: Thu, 15 Jun 2023 09:55:09 +0900

 Sorry for the delayed response!

 On 2023/06/05 18:16, Martin Husemann wrote:
 > On Mon, Jun 05, 2023 at 05:41:55PM +0900, Rin Okuyama wrote:
 >> For crunchgen(1), clear PaX flags instead of removing its section.
 >> Otherwise, a zero-filled hole appears in ELF note segment for arm,
 >> for which MARCH note is located after PaX one.
 > 
 > I still wonder if this should be considered a binutils bug (worth
 > reporting upstream?)

 Well, a candidate fix is:

 (1) If the removed section is located the top or bottom of a segment,
      shrink that segment (as already done).
 (2) Otherwise, split the segment:
 (3) Insert a new program header.
 (4) Relocate a payload backward.
 (5) Adjust {p,sh}_offset and friends appropriately.

 This should be technically possible. But given this has been left
 untouched during the last three decades, they may be not interested...

 >> For this purpose, introduce -0 option for paxctl(8) to zero-clear
 >> PaX flags. IMO, this should be clearer than disabling all available
 >> flags.
 > 
 > I like this idea and the patch looks good to me.

 Thanks for your comment! I will commit soon.

 Thanks,
 rin

From: "Rin Okuyama" <rin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52675 CVS commit: src/usr.sbin/paxctl
Date: Fri, 23 Jun 2023 01:56:22 +0000

 Module Name:	src
 Committed By:	rin
 Date:		Fri Jun 23 01:56:21 UTC 2023

 Modified Files:
 	src/usr.sbin/paxctl: paxctl.8 paxctl.c

 Log Message:
 paxctl(8): Introduce -0 option to clear all PaX flag bits in ELF note.
 Part of PR toolchain/52675


 To generate a diff of this commit:
 cvs rdiff -u -r1.16 -r1.17 src/usr.sbin/paxctl/paxctl.8
 cvs rdiff -u -r1.12 -r1.13 src/usr.sbin/paxctl/paxctl.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Rin Okuyama" <rin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52675 CVS commit: src/usr.bin/crunch/crunchgen
Date: Fri, 23 Jun 2023 02:13:03 +0000

 Module Name:	src
 Committed By:	rin
 Date:		Fri Jun 23 02:13:03 UTC 2023

 Modified Files:
 	src/usr.bin/crunch/crunchgen: crunchgen.c

 Log Message:
 crunchgen(1): Clear PaX flags instead of removing its ELF note section.

 The latter results in zero-filled hole in ELF note segment for EARM,
 where PaX section is not located the bottom of that segment (see
 src/lib/csu/sysident.S). Fortunately, this hole does not cause real
 harms for our in-kernel ELF note parser, except for noisy warnings on
 DIAGNOSTIC kernels.

 Bump CRUNCH_VERSION.

 PR toolchain/52675


 To generate a diff of this commit:
 cvs rdiff -u -r1.94 -r1.95 src/usr.bin/crunch/crunchgen/crunchgen.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->pending-pullups
State-Changed-By: rin@NetBSD.org
State-Changed-When: Fri, 23 Jun 2023 02:48:55 +0000
State-Changed-Why:
[pullup-10 #215] Fix for toolchain/52675                                                                                                                        To be conservative, only for netbsd-10 at the moment.                           Please feel free to request if you offer some basic tests.


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52675 CVS commit: [netbsd-10] src
Date: Tue, 27 Jun 2023 18:20:19 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue Jun 27 18:20:18 UTC 2023

 Modified Files:
 	src/usr.bin/crunch/crunchgen [netbsd-10]: crunchgen.c
 	src/usr.sbin/paxctl [netbsd-10]: paxctl.8 paxctl.c

 Log Message:
 Pull up following revision(s) (requested by rin in ticket #215):

 	usr.sbin/paxctl/paxctl.8: revision 1.17
 	usr.sbin/paxctl/paxctl.8: revision 1.18
 	usr.bin/crunch/crunchgen/crunchgen.c: revision 1.95
 	usr.sbin/paxctl/paxctl.c: revision 1.13

 paxctl(8): Introduce -0 option to clear all PaX flag bits in ELF note.
 Part of PR toolchain/52675

 crunchgen(1): Clear PaX flags instead of removing its ELF note section.
 The latter results in zero-filled hole in ELF note segment for EARM,
 where PaX section is not located the bottom of that segment (see
 src/lib/csu/sysident.S). Fortunately, this hole does not cause real
 harms for our in-kernel ELF note parser, except for noisy warnings on
 DIAGNOSTIC kernels.

 Bump CRUNCH_VERSION.

 PR toolchain/52675

 Use Fl for options.


 To generate a diff of this commit:
 cvs rdiff -u -r1.94 -r1.94.8.1 src/usr.bin/crunch/crunchgen/crunchgen.c
 cvs rdiff -u -r1.16 -r1.16.24.1 src/usr.sbin/paxctl/paxctl.8
 cvs rdiff -u -r1.12 -r1.12.56.1 src/usr.sbin/paxctl/paxctl.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: rin@NetBSD.org
State-Changed-When: Fri, 07 Jul 2023 07:58:18 +0000
State-Changed-Why:
Pulled up to netbsd-10.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2023 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.