fortify-headers: fix inconsistent time_t version of ppoll

[httpstorm]

Bug:
fortify/poll.h includes poll.h, which redirects the ppoll sys call to __ppoll_time64,
if the _REDIR_TIME64 macro is 1. Then fortify/poll.h will #undef ppoll and use the 32 bit version, which is inconsistent.

Fix: we should not do this when _REDIR_TIME64 is 1.

[1] https://forum.openwrt.org/t/idle-cpu-usage-of-usbmuxd/140331/15
[2] #12574

Host tested: macOS 12.6.5
Targets tested: TP-Link TL-WR1043ND v4, WRT3200ACM

@Ansuel @hauke @nbd168 @mpratt14 @1715173329 @jow- @ldir-EDB0

[neheb]

That is...quite the bug.

[httpstorm]

@neheb
In my builds, ppoll is used in usbmuxd, openssh and few other places. Finding all packages that use it would require building or unpacking them. Or perhaps if any one can run grep on the build-bot environment. It's worth checking if other sys calls are affected. I invited the creator.

[httpstorm]

I found two forks on Github: Alpine Linux [1, 2] are aware of the issue and removed ppoll. The @jvoisin fork [3] is still affected.

musl renames the symbol to accommodate for time_t size, and fortify-headers misses that

[1] chimera-linux/fortify-headers@6700b69
[2] alpinelinux/aports@4f60e61
[3] https://github.com/jvoisin/fortify-headers

[hauke]

This looks interesting. Did you report this upstream?

Is it worth to adapt this fortify header check to work with 64 bit time?

[httpstorm]

@hauke

This looks interesting. Did you report this upstream?

Yes, I also invited sin (the author of fortify-headers) to join our discussion, in my e-mail.

Is it worth to adapt this fortify header check to work with 64 bit time?

Likely not. See the links from my previous comment. In [1] they disabled it with#if 0 instead of using #if !_REDIR_TIME64. While in [2] they left the following commit comments:

The fortify-headers/poll.h definitions incorrectly forward the
ppoll() function: on some archs (armv7 for instance), musl
renames the symbol to accommodate for time_t size, and
fortify-headers misses that.
This causes ppoll() to break on all time32 systems.
This commit axes the ppoll() redefinition. Fortifying ppoll()
is of very little benefit anyway.

[hauke]

I would like to backport this to 22.03 soon if we do not see any fallout in master.

For this change to apply we have to recompile the affected applications, we can do this by increasing the PKG_RELEASE of the affected package. We should wit with this about 2 days till all toolchains were rebuild. Do you have a list of packages affected?

I compiled a simple application:

int mytest(struct pollfd *fds, nfds_t nfds)
{
	int err;
	struct timespec time = {0,};

	err = ppoll(fds, nfds, &time, NULL);

	return err;
}

Without this change it results in this:

00401c4c <mytest>:
  401c4c:       27bdffd0        addiu   sp,sp,-48
  401c50:       27a60018        addiu   a2,sp,24
  401c54:       3c022000        lui     v0,0x2000
  401c58:       00003825        move    a3,zero
  401c5c:       afbf002c        sw      ra,44(sp)
  401c60:       afa00018        sw      zero,24(sp)
  401c64:       afa0001c        sw      zero,28(sp)
  401c68:       afa00020        sw      zero,32(sp)
  401c6c:       afa00024        sw      zero,36(sp)
  401c70:       00a20031        tgeu    a1,v0
  401c74:       0c100844        jal     402110 <ppoll@plt>
  401c78:       00000000        nop
  401c7c:       8fbf002c        lw      ra,44(sp)
  401c80:       03e00008        jr      ra
  401c84:       27bd0030        addiu   sp,sp,48
        ...

With this change applied it results in this:

00401c5c <mytest>:
  401c5c:       27bdffd0        addiu   sp,sp,-48
  401c60:       27a60018        addiu   a2,sp,24
  401c64:       00003825        move    a3,zero
  401c68:       afbf002c        sw      ra,44(sp)
  401c6c:       afa00018        sw      zero,24(sp)
  401c70:       afa0001c        sw      zero,28(sp)
  401c74:       afa00020        sw      zero,32(sp)
  401c78:       0c100810        jal     402040 <__ppoll_time64@plt>
  401c7c:       afa00024        sw      zero,36(sp)
  401c80:       8fbf002c        lw      ra,44(sp)
  401c84:       03e00008        jr      ra
  401c88:       27bd0030        addiu   sp,sp,48
  401c8c:       00000000        nop

The ppoll function expects a 32 bit time_t.

[httpstorm]

@hauke

I would like to backport this to 22.03 soon if we do not see any fallout in master.

Do you need me to backport this patch to 22.03 or will you take care of it?
I'll try to monitor this thread for at least a few days. If you ever need me to respond urgently, you know how to find my mail.
With this patch, if a package is compiled with 64 bit time_t, then _REDIR_TIME64 = 1 and fortify-headers are bypassed. Else the old code is preserved. So hopefully no fallout should be observed.

For this change to apply we have to recompile the affected applications, we can do this by increasing the PKG_RELEASE of the affected package. We should wit with this about 2 days till all toolchains were rebuild. Do you have a list of packages affected?

Can you tell the build-bot to rebuild everything? Does it happen periodically and how often?
In order for me to grep the source from all packages, they have to be unpacked. Is there a way to prepare all packages without building them? At the moment, I can only grep those that are compiled for my router. ppoll is used in only a few of them. openssh and uxbmuxd being the most important. I remember there was some menuconfig setting that causes all packages to get build. But I don't remember the name or where it is. If you remind me, I can enable it and try to build everything. I know from experience that many packages will fail to compile on a macOS host, so that's not a good option. I can also try building on my 13 year old i7 930 desktop computer. It has an Ubuntu 22.04 VM. Or if you can run grep on the build-bot where all packages are already compiled this will be the best way to find all packages that use ppoll.

I compiled a simple application:

I have experience with ARM assembler. Good enough that I wrote nano RTOS. But the code you listed is for another CPU and I don't understand it. Perhaps if you compile it for WRT3200ACM? I can also try building your C code, but right now I need to finish a review for a patch to the ipheth iPhone network driver for Linux. And I lack experience when it comes to USB CDC NCM, so I expect this to take my whole day.

[httpstorm]

@hauke
I did not consider some targets do not define the _REDIR_TIME64 macro resulting in a build error regression.
Fixed by checking if the macro is defined here #12590
Currently running a test build…

[httpstorm]

@hauke
Regression fixed in my new PR #12590

[hauke]

@httpstorm

@hauke

I would like to backport this to 22.03 soon if we do not see any fallout in master.

Do you need me to backport this patch to 22.03 or will you take care of it? I'll try to monitor this thread for at least a few days. If you ever need me to respond urgently, you know how to find my mail. With this patch, if a package is compiled with 64 bit time_t, then _REDIR_TIME64 = 1 and fortify-headers are bypassed. Else the old code is preserved. So hopefully no fallout should be observed.

I was talking about this fallout. ;-)
I will cherry-pick both commits to 22.03 in the next few days, you do not have to do anything if I do not forget it.

For this change to apply we have to recompile the affected applications, we can do this by increasing the PKG_RELEASE of the affected package. We should wit with this about 2 days till all toolchains were rebuild. Do you have a list of packages affected?

Can you tell the build-bot to rebuild everything? Does it happen periodically and how often? In order for me to grep the source from all packages, they have to be unpacked. Is there a way to prepare all packages without building them? At the moment, I can only grep those that are compiled for my router. ppoll is used in only a few of them. openssh and uxbmuxd being the most important. I remember there was some menuconfig setting that causes all packages to get build. But I don't remember the name or where it is. If you remind me, I can enable it and try to build everything. I know from experience that many packages will fail to compile on a macOS host, so that's not a good option. I can also try building on my 13 year old i7 930 desktop computer. It has an Ubuntu 22.04 VM. Or if you can run grep on the build-bot where all packages are already compiled this will be the best way to find all packages that use ppoll.

The build bot will automatically rebuild all packages, but opkg will not update them as log as the version did not change.
This recompilation should happen every 2 days.

When I find some time I will build all packages from the feed and grep for ppoll. We should anyway wait till the SDKs were build.

I compiled a simple application:

I have experience with ARM assembler. Good enough that I wrote nano RTOS. But the code you listed is for another CPU and I don't understand it. Perhaps if you compile it for WRT3200ACM? I can also try building your C code, but right now I need to finish a review for a patch to the ipheth iPhone network driver for Linux. And I lack experience when it comes to USB CDC NCM, so I expect this to take my whole day.

This was for MIPS32r2 CPU. The new code now links the 64 bit version of the function.

[httpstorm]

@hauke

I was talking about this fallout. ;-)

Nice instincts. I am learning to appreciate the tight compiler options because it allows issues to be identified earlier.
In my projects undefined macros always translate to 0, which is convenient, but as it seems does not provide strict behaviour when it comes to 3rd party headers.

I will cherry-pick both commits to 22.03 in the next few days, you do not have to do anything if I do not forget it.

ok

The build bot will automatically rebuild all packages, but opkg will not update them as log as the version did not change.
This recompilation should happen every 2 days.
When I find some time I will build all packages from the feed and grep for ppoll. We should anyway wait till the SDKs were build.

I learned about the make download and IGNORE_ERRORS=1 last night. Then I can unpack all archives and search for ppoll. I'll give it a try.

This was for MIPS32r2 CPU. The new code now links the 64 bit version of the function.

Thanks for confirming!

[httpstorm]

@hauke
List of all packages using ppoll:
https://httpstorm.com/share/.openwrt/test/2023-05-15_ppoll/ppoll-2023-05-15.txt