Hello folks,
am I allowed to override -D_FORTIFY_SOURCE=2 with -D_FORTIFY_SOURCE=0 for my Zarafa package until either upstream fixed their code or until glibc is making their protection check more flexible? I already read the Packaging Guidelines, but they still leave open, what's happening, if I am doing it.
Our glibc > 2.14.90-8 contains a commit which adds range checking to FD_SET and friends (http://sourceware.org/ml/glibc-cvs/2011-q3/msg00235.html). I also found http://sourceware.org/bugzilla/show_bug.cgi?id=10352 meanwhile, and it's funny to see that Ulrich Drepper implemented what he didn't even want to revisit in the bug report.
Finally, that FD_SET range checking causes Zarafa to segfault once it has been built using glibc > 2.14.90-8 (which is Fedora 16+), __fdelt_chk() is simply called as they use more than 1024 file descriptors.
https://bugzilla.redhat.com/show_bug.cgi?id=760888 contains whole story, I sat down with Jan Kratochvil to identify the cause, communicated with the Zarafa developers and with Jeff Law, our glibc package maintainer. And also with Adam Jackson on IRC.
The feedback from the Zarafa developers was, that the check in glibc is a good idea, but based on wrong assumptions. Together with the upcoming mass- rebuild they think, we could have a lot of fun, because many software will possibly run into __fdelt_chk(). I hope, I summarized them correct.
I'm not a developer/programmer, but from what I get > 1024 file descriptors work under certain conditions and if correctly developed and thus the glibc range check is maybe not flexible enough.
From the BSD 4.2 FD_SET man page (http://www.manpagez.com/man/2/FD_SET/):
[...] The default size FD_SETSIZE (currently 1024) is somewhat smaller than the current kernel limit to the number of open files. However, in order to accommodate programs which might potentially use a larger number of open files with select, it is possible to increase this size within a program by providing a larger definition of FD_SETSIZE before the inclusion of <sys/types.h>. [...]
I might be wrong, but that is what Zarafa is doing and what glibc code is not covering. Maybe somebody with proper knowledge can have a look to this once more? Thanks!
# Segmentation faults with glibc > 2.14.90-8, see RHBZ #760888 export CXXFLAGS="${RPM_OPT_FLAGS/-D_FORTIFY_SOURCE=2/-D_FORTIFY_SOURCE=0}"
Above is the snippet, that I would like to introduce into the spec file for the time being and to get the software up and running again. As you can see in the bug report, there are users running into this issue.
Greetings, Robert
Robert Scheck robert@fedoraproject.org writes:
[ _FORTIFY_SOURCE=2 breaks non-default FD_SETSIZE settings ]
So far as I can see from the sys/select.h header file, there is no intention for glibc to support overriding the value of FD_SETSIZE; certainly, doing so doesn't affect sizeof(struct fd_set). It's possible that it'd work if you don't use that struct declaration, but then you're definitely outside the boundaries of what can be called portable code. So while you'll probably want an authoritative opinion from a glibc maintainer, I'd venture that overriding FD_SETSIZE is a nonportable BSD-ism.
I think the reason this hasn't been complained of too much is that it's generally better to use poll(2) instead of select(2) if your program can have a lot of file descriptors open. Have the Zarafa developers considered offering a poll()-based option?
regards, tom lane
Hello Tom,
On Mon, 09 Jan 2012, Tom Lane wrote:
I think the reason this hasn't been complained of too much is that it's generally better to use poll(2) instead of select(2) if your program can have a lot of file descriptors open. Have the Zarafa developers considered offering a poll()-based option?
I have taken that topic already to Zarafa, more than void was not yet returned, however there is an internal developer meeting this week, I think.
Even if they decide to rewrite the code, it's not done immediately and non-paid code rewrites maybe also take some time, it's similar like at RHEL vs. subscription, if I'm allowed to compare.
Would -D_FORTIFY_SOURCE=0 be acceptable until the code is rewritten?
Greetings, Robert
On Tuesday 10 January 2012 11:25:39 Robert Scheck wrote:
Hello Tom,
On Mon, 09 Jan 2012, Tom Lane wrote:
I think the reason this hasn't been complained of too much is that it's generally better to use poll(2) instead of select(2) if your program can have a lot of file descriptors open. Have the Zarafa developers considered offering a poll()-based option?
I have taken that topic already to Zarafa, more than void was not yet returned, however there is an internal developer meeting this week, I think.
Even if they decide to rewrite the code, it's not done immediately and non-paid code rewrites maybe also take some time, it's similar like at RHEL vs. subscription, if I'm allowed to compare.
Would -D_FORTIFY_SOURCE=0 be acceptable until the code is rewritten?
If you prefer undefined behavior over valid assertion failure, then yes.
Kamil
On Tue, Jan 10, 2012 at 11:25:39AM +0100, Robert Scheck wrote:
Hello Tom,
On Mon, 09 Jan 2012, Tom Lane wrote:
I think the reason this hasn't been complained of too much is that it's generally better to use poll(2) instead of select(2) if your program can have a lot of file descriptors open. Have the Zarafa developers considered offering a poll()-based option?
I have taken that topic already to Zarafa, more than void was not yet returned, however there is an internal developer meeting this week, I think.
Even if they decide to rewrite the code, it's not done immediately and non-paid code rewrites maybe also take some time, it's similar like at RHEL vs. subscription, if I'm allowed to compare.
Would -D_FORTIFY_SOURCE=0 be acceptable until the code is rewritten?
As Tom pointed out, if you override FD_SETSIZE with glibc, this has no effect on the size of the 'fd_set' struct. So any attempt to actually store a larger number of FDs will be writing outside the bounds of the struct. ie it will be corrupting heap/stack memory. This is the kind of flaw that leads to crashes at best, or security exploits at worst.
Thus, IMHO, it is not acceptable to set -D_FORTIFY_SOURCE=0. You'd be building known broken, potentially insecure binaries.
Regards, Daniel
"Daniel P. Berrange" berrange@redhat.com writes:
On Tue, Jan 10, 2012 at 11:25:39AM +0100, Robert Scheck wrote:
Would -D_FORTIFY_SOURCE=0 be acceptable until the code is rewritten?
As Tom pointed out, if you override FD_SETSIZE with glibc, this has no effect on the size of the 'fd_set' struct. So any attempt to actually store a larger number of FDs will be writing outside the bounds of the struct. ie it will be corrupting heap/stack memory. This is the kind of flaw that leads to crashes at best, or security exploits at worst.
Perhaps a more reliable workaround would be to patch in some code at program start that reduces the soft limit on number of open files to 1K or less (see setrlimit(RLIMIT_NOFILE)). This would presumably reduce performance by some fractional amount, but that seems better than the unsafe behavior you're looking at now.
regards, tom lane
Hello Tom,
On Mon, 09 Jan 2012, Tom Lane wrote:
I think the reason this hasn't been complained of too much is that it's generally better to use poll(2) instead of select(2) if your program can have a lot of file descriptors open. Have the Zarafa developers considered offering a poll()-based option?
meanwhile, I got the reply that --enable-epoll triggers epoll(2) usage. However I am already building with that ./configure option and thus it is not solving the issue. Does it mean there are other select(2) overlefts?
Greetings, Robert
packaging@lists.fedoraproject.org
-
Daniel P. Berrange -
Kamil Dudka -
Robert Scheck -
Tom Lane