Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 12 May 2023 18:39:47 +0000
From:   Fangrui Song <maskray@google.com>
To:     Kees Cook <keescook@chromium.org>
Cc:     Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>,
        Eric Biederman <ebiederm@xmission.com>,
        Baoquan He <bhe@redhat.com>, Vivek Goyal <vgoyal@redhat.com>,
        Dave Young <dyoung@redhat.com>, linux-fsdevel@vger.kernel.org,
        kexec@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] coredump, vmcore: Set p_align to 4 for PT_NOTE
Message-ID: <20230512183947.jvaslvmuhy7gndix@google.com>
References: <20230512022528.3430327-1-maskray@google.com>
 <202305121126.E5AD334AA3@keescook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <202305121126.E5AD334AA3@keescook>
Precedence: bulk

On 2023-05-12, Kees Cook wrote:
>On Fri, May 12, 2023 at 02:25:28AM +0000, Fangrui Song wrote:
>> Tools like readelf/llvm-readelf use p_align to parse a PT_NOTE program
>> header as an array of 4-byte entries or 8-byte entries. Currently, there
>> are workarounds[1] in place for Linux to treat p_align==0 as 4. However,
>> it would be more appropriate to set the correct alignment so that tools
>> do not have to rely on guesswork. FreeBSD coredumps set p_align to 4 as
>> well.
>>
>> [1]: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=82ed9683ec099d8205dc499ac84febc975235af6
>
>The interesting bit from here is:
>
>  /* NB: Some note sections may have alignment value of 0 or 1.  gABI
>     specifies that notes should be aligned to 4 bytes in 32-bit
>     objects and to 8 bytes in 64-bit objects.  As a Linux extension,
>     we also support 4 byte alignment in 64-bit objects.  If section
>     alignment is less than 4, we treate alignment as 4 bytes.   */
>  if (align < 4)
>    align = 4;
>  else if (align != 4 && align != 8)
>    {
>      warn (_("Corrupt note: alignment %ld, expecting 4 or 8\n"),
>           (long) align);
>      return FALSE;
>    }
>
>Should Linux use 8 for 64-bit processes to avoid the other special case?
>
>(And do we need to make some changes to make sure we are actually
>aligned?)
>
>-Kees

64-bit objects should use 8-byte entries and naturally the 8-byte alignment.
Unfortunately, many systems including Solaris, *BSD, and Linux use
4-byte entries for SHT_NOTE/PT_NOTE, and changing this will create
a large compatibility problem (see tcmalloc that I recently
updated[1])

Linux introduced 8-byte alignment note sections (.note.gnu.property) a
while ago, so the ecosystem has to deal with notes of mixed alignments.
The resolution is to use the note alignment to decide whether it should
be parsed as 4-byte entries or 8-byte entries.
I think that just setting `p_align = 4` on the kernel side should be
good enough:)

[1]:
https://github.com/google/tcmalloc/commit/c33cb2d8935002f8ba942028a1f0871d075345a1