Re: [RFC PATCH 05/28] x86: Define the stack protector guard symbol explicitly

[Brian Gerst]

On Fri, Oct 4, 2024 at 9:15 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> On Sat, 28 Sept 2024 at 15:41, Brian Gerst <brgerst@xxxxxxxxx> wrote:
> >
> > On Wed, Sep 25, 2024 at 2:33 PM Uros Bizjak <ubizjak@xxxxxxxxx> wrote:
> > >
> > > On Wed, Sep 25, 2024 at 5:02 PM Ard Biesheuvel <ardb+git@xxxxxxxxxx> wrote:
> > > >
> > > > From: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > >
> > > > Specify the guard symbol for the stack cookie explicitly, rather than
> > > > positioning it exactly 40 bytes into the per-CPU area. Doing so removes
> > > > the need for the per-CPU region to be absolute rather than relative to
> > > > the placement of the per-CPU template region in the kernel image, and
> > > > this allows the special handling for absolute per-CPU symbols to be
> > > > removed entirely.
> > > >
> > > > This is a worthwhile cleanup in itself, but it is also a prerequisite
> > > > for PIE codegen and PIE linking, which can replace our bespoke and
> > > > rather clunky runtime relocation handling.
> > >
> > > I would like to point out a series that converted the stack protector
> > > guard symbol to a normal percpu variable [1], so there was no need to
> > > assume anything about the location of the guard symbol.
> > >
> > > [1] "[PATCH v4 00/16] x86-64: Stack protector and percpu improvements"
> > > https://lore.kernel.org/lkml/20240322165233.71698-1-brgerst@xxxxxxxxx/
> > >
> > > Uros.
> >
> > I plan on resubmitting that series sometime after the 6.12 merge
> > window closes.  As I recall from the last version, it was decided to
> > wait until after the next LTS release to raise the minimum GCC version
> > to 8.1 and avoid the need to be compatible with the old stack
> > protector layout.
> >
>
> Hi Brian,
>
> I'd be more than happy to compare notes on that - I wasn't aware of
> your intentions here, or I would have reached out before sending this
> RFC.
>
> There are two things that you would need to address for Clang support
> to work correctly:
> - the workaround I cc'ed you on the other day [0],
> - a workaround for the module loader so it tolerates the GOTPCRELX
> relocations that Clang emits [1]
>
>
>
> [0] https://lore.kernel.org/all/20241002092534.3163838-2-ardb+git@xxxxxxxxxx/
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/commit/?id=a18121aabbdd

The first patch should be applied independently as a bug fix, since it
already affects the 32-bit build with clang.

I don't have an environment with an older clang compiler to test the
second patch, but I'll assume it will be necessary.  I did run into an
issue with the GOTPCRELX relocations before [1], but I thought it was
just an objtool issue and didn't do more testing to know if modules
were broken or not.

Brian Gerst

[1] https://lore.kernel.org/all/20231026160100.195099-6-brgerst@xxxxxxxxx/