ftrace: sorttable unable to sort ELF64 on 32-bit host

ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

Hi Steven,

On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts
because of the parsing of the start_mcount_loc and stop_mcount_loc
values in get_mcount_loc():

  *_start = strtoul(start_buff, NULL, 16);

and

 *_stop = strtoul(stop_buff, NULL, 16);

This code makes the (often correct) assumption that the host and the
target have the same architecture, however it runs into issues when
compiling for a 64-bit target on a 32-bit host, as unsigned long is
shorter than the pointer width. As a result, I've noticed that both
start and stop max out at 2^32 - 1.

It seems that commit 4acda8ed fixes this issue inadvertently by
directly extracting them from the ELF using the correct width. I'm
wondering if it is possible to backport this as well as the other
sorttable refactors to 6.12.0 since they fix this issue.

Thanks,
Sahil

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Thu, 20 Mar 2025 17:02:06 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> Hi Steven,

Hi,

Sorry for the really late reply. I'm cleaning out my inbox and just noticed
this email.

> 
> On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts
> because of the parsing of the start_mcount_loc and stop_mcount_loc
> values in get_mcount_loc():
> 
>   *_start = strtoul(start_buff, NULL, 16);
> 
> and
> 
>  *_stop = strtoul(stop_buff, NULL, 16);
> 
> This code makes the (often correct) assumption that the host and the
> target have the same architecture, however it runs into issues when
> compiling for a 64-bit target on a 32-bit host, as unsigned long is
> shorter than the pointer width. As a result, I've noticed that both
> start and stop max out at 2^32 - 1.
> 
> It seems that commit 4acda8ed fixes this issue inadvertently by
> directly extracting them from the ELF using the correct width. I'm
> wondering if it is possible to backport this as well as the other
> sorttable refactors to 6.12.0 since they fix this issue.

I think this is a good reason to mark that commit as stable and backport it
to 6.12.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

Sounds good, many thanks.

Sahil

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Thu, 20 Mar 2025 16:54:58 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> Hi Steven,
> 
> On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> of the parsing of the start_mcount_loc and stop_mcount_loc values in
> get_mcount_loc():
> 
>   *_start = strtoul(start_buff, NULL, 16);
> 
> and
> 
>  *_stop = strtoul(stop_buff, NULL, 16);
> 
> This code makes the (often correct) assumption that the host and the target
> have the same architecture, however it runs into issues when compiling for
> a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> pointer width. As a result, I've noticed that both start and stop max out
> at 2^32 - 1.

So this has been broken for some time?

> 
> It seems that commit 4acda8ed fixes this issue inadvertently by directly
> extracting them from the ELF using the correct width. I'm wondering if it
> is possible to backport this as well as the other sorttable refactors to
> 6.12.0 since they fix this issue.

You should ask Greg KH on this.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

> > Hi Steven,
> >
> > On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> > of the parsing of the start_mcount_loc and stop_mcount_loc values in
> > get_mcount_loc():
> >
> >   *_start = strtoul(start_buff, NULL, 16);
> >
> > and
> >
> >  *_stop = strtoul(stop_buff, NULL, 16);
> >
> > This code makes the (often correct) assumption that the host and the target
> > have the same architecture, however it runs into issues when compiling for
> > a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> > pointer width. As a result, I've noticed that both start and stop max out
> > at 2^32 - 1.
>
> So this has been broken for some time?

Since 2021, it seems like. However, this slipped from my radar since
we've been using 5.10 for the longest time, which didn't have
CONFIG_BUILDTIME_MCOUNT_SORT.

> >
> > It seems that commit 4acda8ed fixes this issue inadvertently by directly
> > extracting them from the ELF using the correct width. I'm wondering if it
> > is possible to backport this as well as the other sorttable refactors to
> > 6.12.0 since they fix this issue.
>
> You should ask Greg KH on this.

Sounds good, tagging Greg.

Sahil


On Mon, Mar 24, 2025 at 12:35 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 20 Mar 2025 16:54:58 -0500
> Sahil Gupta <s.gupta@arista.com> wrote:
>
> > Hi Steven,
> >
> > On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> > of the parsing of the start_mcount_loc and stop_mcount_loc values in
> > get_mcount_loc():
> >
> >   *_start = strtoul(start_buff, NULL, 16);
> >
> > and
> >
> >  *_stop = strtoul(stop_buff, NULL, 16);
> >
> > This code makes the (often correct) assumption that the host and the target
> > have the same architecture, however it runs into issues when compiling for
> > a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> > pointer width. As a result, I've noticed that both start and stop max out
> > at 2^32 - 1.
>
> So this has been broken for some time?
>
> >
> > It seems that commit 4acda8ed fixes this issue inadvertently by directly
> > extracting them from the ELF using the correct width. I'm wondering if it
> > is possible to backport this as well as the other sorttable refactors to
> > 6.12.0 since they fix this issue.
>
> You should ask Greg KH on this.
>
> -- Steve
>

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Greg KH]

On Mon, Mar 24, 2025 at 01:39:12PM -0500, Sahil Gupta wrote:
> > > Hi Steven,
> > >
> > > On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> > > of the parsing of the start_mcount_loc and stop_mcount_loc values in
> > > get_mcount_loc():
> > >
> > >   *_start = strtoul(start_buff, NULL, 16);
> > >
> > > and
> > >
> > >  *_stop = strtoul(stop_buff, NULL, 16);
> > >
> > > This code makes the (often correct) assumption that the host and the target
> > > have the same architecture, however it runs into issues when compiling for
> > > a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> > > pointer width. As a result, I've noticed that both start and stop max out
> > > at 2^32 - 1.
> >
> > So this has been broken for some time?
> 
> Since 2021, it seems like. However, this slipped from my radar since
> we've been using 5.10 for the longest time, which didn't have
> CONFIG_BUILDTIME_MCOUNT_SORT.
> 
> > >
> > > It seems that commit 4acda8ed fixes this issue inadvertently by directly
> > > extracting them from the ELF using the correct width. I'm wondering if it
> > > is possible to backport this as well as the other sorttable refactors to
> > > 6.12.0 since they fix this issue.
> >
> > You should ask Greg KH on this.
> 
> Sounds good, tagging Greg.

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Mon, 24 Mar 2025 11:52:33 -0700
Greg KH <gregkh@linuxfoundation.org> wrote:

> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Hi Greg,

This isn't a patch submission. It's more of a question if something should
be backported because it happens to fix a long standing issue.

It appears that the ftrace mcount sorttable code was broken if you built a
64 bit kernel on a 32 bit machine. I'm guessing it was broken since 5.17?
A recent update to this code coincidentally fixes that issue. That update
landed in 6.14. The question is, is it fine to backport the changes that
fix this? It may not be totally trivial to do so, but I could likely just
backport the changes that address this issue.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Greg KH]

On Mon, Mar 24, 2025 at 03:07:03PM -0400, Steven Rostedt wrote:
> On Mon, 24 Mar 2025 11:52:33 -0700
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> >     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> Hi Greg,
> 
> This isn't a patch submission. It's more of a question if something should
> be backported because it happens to fix a long standing issue.

Sorry, that wasn't obvious to me.

> It appears that the ftrace mcount sorttable code was broken if you built a
> 64 bit kernel on a 32 bit machine. I'm guessing it was broken since 5.17?
> A recent update to this code coincidentally fixes that issue. That update
> landed in 6.14. The question is, is it fine to backport the changes that
> fix this? It may not be totally trivial to do so, but I could likely just
> backport the changes that address this issue.

It's up to the maintainer of the subsytem as to what they wish to see
happen.  I will always defer to them as they are the ones that have to
deal with emails from users :)

thanks,

greg k-h

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Tue, 25 Mar 2025 07:58:36 -0400
Greg KH <gregkh@linuxfoundation.org> wrote:

> It's up to the maintainer of the subsytem as to what they wish to see
> happen.  I will always defer to them as they are the ones that have to
> deal with emails from users :)

Thanks Greg for the reply.

Now bringing the question to Sahil.

This looks to be broken in 5.17 (when the mcount sorting was added). And
you are now using 6.12. How important is it for you to build 64 bit kernels
on 32 bit machines?

Nobody noticed this for a long time. One solution may be to simply disable
mcount sorting at build time when it is detected that the host is 32 bit
and the target is 64 bit. I don't know how easy it is to detect that at
build time, as it needs to unset a kernel CONFIG option. Perhaps Kconfig
can detect that?

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

On Tue, 25 Mar 2025 05:15:10 -0700 (PDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> How important is it for you to build 64 bit kernels
> on 32 bit machines?

Yeah I don't disagree at all, it is a fairly idiosyncratic thing to
do. There are some historical and business justifications for still
building 32-bit in the first place, but until we deduplicate the
kernel build, we will continue to have this issue.

> One solution may be to simply disable
> mcount sorting at build time when it is detected that the host is 32 bit
> and the target is 64 bit.

We are currently doing this. I imagine the performance difference is
trivial but if we can sort at build time, we might as well. I have a
fairly simple alternate solution that isn't backporting all of those
patches. The core idea is to introduce another macro, parse_addr, that
is defined as

 # define parse_addr(buf) strtoull(buf, NULL, 16)

when SORTTABLE_64 is defined and

 # define parse_addr(buf) strtoul(buf, NULL, 16)

when it isn't. Seems like a fair thing to do considering unsigned long
long is guaranteed to be at least 64-bit and unsigned long is
guaranteed to be at least 32-bit. I can post the patch in a stable-
thread.

Sahil

On Tue, Mar 25, 2025 at 7:15 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 07:58:36 -0400
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > It's up to the maintainer of the subsytem as to what they wish to see
> > happen.  I will always defer to them as they are the ones that have to
> > deal with emails from users :)
>
> Thanks Greg for the reply.
>
> Now bringing the question to Sahil.
>
> This looks to be broken in 5.17 (when the mcount sorting was added). And
> you are now using 6.12. How important is it for you to build 64 bit kernels
> on 32 bit machines?
>
> Nobody noticed this for a long time. One solution may be to simply disable
> mcount sorting at build time when it is detected that the host is 32 bit
> and the target is 64 bit. I don't know how easy it is to detect that at
> build time, as it needs to unset a kernel CONFIG option. Perhaps Kconfig
> can detect that?
>
> -- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Tue, 25 Mar 2025 12:52:43 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> On Tue, 25 Mar 2025 05:15:10 -0700 (PDT)
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > How important is it for you to build 64 bit kernels
> > on 32 bit machines?  
> 
> Yeah I don't disagree at all, it is a fairly idiosyncratic thing to
> do. There are some historical and business justifications for still
> building 32-bit in the first place, but until we deduplicate the
> kernel build, we will continue to have this issue.

Note, that the regression is only the failure in the build of your setup.
As your setup never supported sorting the mcount location at build time,
that part is not the regression.

> 
> > One solution may be to simply disable
> > mcount sorting at build time when it is detected that the host is 32 bit
> > and the target is 64 bit.  
> 
> We are currently doing this. I imagine the performance difference is
> trivial but if we can sort at build time, we might as well. I have a
> fairly simple alternate solution that isn't backporting all of those
> patches. The core idea is to introduce another macro, parse_addr, that
> is defined as
> 
>  # define parse_addr(buf) strtoull(buf, NULL, 16)
> 
> when SORTTABLE_64 is defined and
> 
>  # define parse_addr(buf) strtoul(buf, NULL, 16)
> 
> when it isn't. Seems like a fair thing to do considering unsigned long
> long is guaranteed to be at least 64-bit and unsigned long is
> guaranteed to be at least 32-bit. I can post the patch in a stable-
> thread.

I'm not against you doing that, but it is specific for your setup.

If you do this, make sure to test it by enabling:

  CONFIG_FTRACE_SORT_STARTUP_TEST

Which will verify that the table is indeed sorted at run time. There could
be another bug that could make it build, but not sort it properly, and
without that config, you may not know, but it will cause ftrace to not work
properly.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

On Tue, 25 Mar 2025 11:01:41 -0700 (PDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> I'm not against you doing that, but it is specific for your setup.
>
> If you do this, make sure to test it by enabling:
>
>   CONFIG_FTRACE_SORT_STARTUP_TEST
>
> Which will verify that the table is indeed sorted at run time. There could
> be another bug that could make it build, but not sort it properly, and
> without that config, you may not know, but it will cause ftrace to not work
> properly.

That sounds good to me. I didn't use CONFIG_FTRACE_SORT_STARTUP_TEST,
but I manually verified the section was sorted by checking the ELF
using a Python script. Maybe that is something we can consider adding
as a build step in the future?

Sahil


On Tue, Mar 25, 2025 at 1:01 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 12:52:43 -0500
> Sahil Gupta <s.gupta@arista.com> wrote:
>
> > On Tue, 25 Mar 2025 05:15:10 -0700 (PDT)
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > How important is it for you to build 64 bit kernels
> > > on 32 bit machines?
> >
> > Yeah I don't disagree at all, it is a fairly idiosyncratic thing to
> > do. There are some historical and business justifications for still
> > building 32-bit in the first place, but until we deduplicate the
> > kernel build, we will continue to have this issue.
>
> Note, that the regression is only the failure in the build of your setup.
> As your setup never supported sorting the mcount location at build time,
> that part is not the regression.
>
> >
> > > One solution may be to simply disable
> > > mcount sorting at build time when it is detected that the host is 32 bit
> > > and the target is 64 bit.
> >
> > We are currently doing this. I imagine the performance difference is
> > trivial but if we can sort at build time, we might as well. I have a
> > fairly simple alternate solution that isn't backporting all of those
> > patches. The core idea is to introduce another macro, parse_addr, that
> > is defined as
> >
> >  # define parse_addr(buf) strtoull(buf, NULL, 16)
> >
> > when SORTTABLE_64 is defined and
> >
> >  # define parse_addr(buf) strtoul(buf, NULL, 16)
> >
> > when it isn't. Seems like a fair thing to do considering unsigned long
> > long is guaranteed to be at least 64-bit and unsigned long is
> > guaranteed to be at least 32-bit. I can post the patch in a stable-
> > thread.
>
> I'm not against you doing that, but it is specific for your setup.
>
> If you do this, make sure to test it by enabling:
>
>   CONFIG_FTRACE_SORT_STARTUP_TEST
>
> Which will verify that the table is indeed sorted at run time. There could
> be another bug that could make it build, but not sort it properly, and
> without that config, you may not know, but it will cause ftrace to not work
> properly.
>
> -- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Tue, 25 Mar 2025 13:10:17 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> That sounds good to me. I didn't use CONFIG_FTRACE_SORT_STARTUP_TEST,
> but I manually verified the section was sorted by checking the ELF
> using a Python script. Maybe that is something we can consider adding
> as a build step in the future?

Note, that config only enables the runtime verification. Which means you
need to run the kernel for the check if it worked. It is not a build time
feature. And since it checks at run time, it does add a small overhead (to
iterate all functions), where you may not want it enabled in production.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

On Tue, 25 Mar 2025 11:18:52 -0700 (PDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> Note, that config only enables the runtime verification. Which means you
> need to run the kernel for the check if it worked. It is not a build time
> feature. And since it checks at run time, it does add a small overhead (to
> iterate all functions), where you may not want it enabled in production.

Right, sounds good. I'll test it with CONFIG_FTRACE_SORT_STARTUP_TEST
and send the patch via a stable- thread if it succeeds.

Sahil


On Tue, Mar 25, 2025 at 1:18 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 13:10:17 -0500
> Sahil Gupta <s.gupta@arista.com> wrote:
>
> > That sounds good to me. I didn't use CONFIG_FTRACE_SORT_STARTUP_TEST,
> > but I manually verified the section was sorted by checking the ELF
> > using a Python script. Maybe that is something we can consider adding
> > as a build step in the future?
>
> Note, that config only enables the runtime verification. Which means you
> need to run the kernel for the check if it worked. It is not a build time
> feature. And since it checks at run time, it does add a small overhead (to
> iterate all functions), where you may not want it enabled in production.
>
> -- Steve