Bug 25718 - APR's sendfile revision detection for FreeBSD is broken
Summary: APR's sendfile revision detection for FreeBSD is broken
Status: RESOLVED FIXED
Alias: None
Product: APR
Classification: Unclassified
Component: APR (show other bugs)
Version: HEAD
Hardware: PC FreeBSD
: P3 normal (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
URL:
Keywords: PatchAvailable
Depends on:
Blocks: 29858
  Show dependency tree
 
Reported: 2003-12-23 07:53 UTC by Mike Silbersack
Modified: 2005-03-13 21:35 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Silbersack 2003-12-23 07:53:41 UTC
Apache (really apr, I guess) attempts to be clever and detect the version
of FreeBSD at runtime; this information is then used to determine whether
or not the length of headers needs to be added to the length passed to
sendfile.  This is to work around a bug present before 4.6-stable which
required the length to be passed.

Anyway, this entire check is unneeded.  As part of the fix, alfred perlstein
created a new sendfile syscall; any program compiled with < 4.6 header files
will use the old syscall which requires the header length to be added in, and
any program compiled with > 4.6 will call the new syscall.  Hence, only a
compile time check should be done.  The current runtime test is basically
one huge no-op.  Patch will be attached in a second.
Comment 1 Mike Silbersack 2003-12-23 07:55:54 UTC
Patch is available from:
http://www.silby.com/patches/raw/patch-sendfile_fix

This should apply to any 2.x version of apache, I believe.
Comment 2 Paul Querna 2004-02-04 01:47:19 UTC
+1

Tested on FreeBSD 5.2

Also Moving this to the APR Project. (Not an Apache Bug)
Comment 3 Joe Orton 2004-02-04 07:10:54 UTC
Has a binary built on >4.6 been tested on <4.6?
Comment 4 Mike Silbersack 2004-02-04 07:25:59 UTC
Yes, a > 4.6 binary dies when run on pre-4.6, as it uses a syscall unknown to
the pre-4.6 kernel.  It would be possible to rig apache so that it actually knew
which sendfile (new or old) to use so that post-4.6 binaries would work on older
kernels, but there's really no reason to bother.
Comment 5 Jeff Trawick 2004-02-04 14:23:06 UTC
Yes, we shouldn't care if a binary built on OS level x fails when run on lower
level of the OS.

Has anybody tested this on older FreeBSD?

(I remember the pain when www.apache.org got screwed by picking up a FreeBSD
update that changed the sendfile() semantics.  There was no compatibility
sendfile() interface at the time to make the old Apache code work.  Maybe it is
only some intermediate maintenance levels of 4.5(?) that will break without the
code in cvs today???)
Comment 6 Joe Orton 2004-02-04 14:28:12 UTC
Yeah, sorry, I meant the other way round: has a binary built with this change on
old FreeBSD been tested to DTRT on new FreeBSD too.
Comment 7 Mike Silbersack 2004-02-07 07:41:30 UTC
I have not tested a pre-4.6 binary, as I do not have any pre-4.6 machines
sitting around.  I assume that this was well tested when the osendfile
comaptibility shim was added.
Comment 8 Paul Querna 2004-07-09 08:50:26 UTC
Do we need to test for a Binary Built on <4.6 working on >=4.6?

If not, is there any reason that we cannot commit this patch?

If this patch is applied, we would also fix the sendfile detection on DragonFly
BSD (Bug #29858)

-Paul Querna
Comment 9 Paul Querna 2004-07-09 20:22:55 UTC
Using APR Built On FreeBSD 4.5(thanks VMWare) and the supplied patch, All Tests
pass. (including ./sendilfe server &; ./sendfile client
timeout,nonblocking,blocking)

Taking the APR built on FreeBSD 4.5, to a 5.2-CURRENT box, the same tests all
pass, including the sendfile tests.

There is no reason to not include the patch now.
Comment 10 Garrett Rooney 2004-11-18 05:27:42 UTC
is there anything keeping this patch from being committed?

the last comment is from four months ago and indicates that testing on the
various versions of freebsd worked out fine, but as far as I can tell the
version in trunk is still using sysctl to figure this stuff out at runtime.
Comment 11 Paul Querna 2004-11-18 05:30:08 UTC
I will commit it this weekend to TRUNK.

-Paul Querna
Comment 12 Paul Querna 2004-11-28 22:55:12 UTC
Committed to trunk as svn revision 106850.
Comment 13 YONETANI Tomokazu 2004-12-22 02:55:58 UTC
(In reply to comment #12)
> Committed to trunk as svn revision 106850.

Thanks! Is there an expected date when this fix also gets
backported to release branches? The fix also (partially)
resolves Bug 29858.
Comment 14 Paul Querna 2004-12-22 05:38:52 UTC
I wasn't planning to backport this to apr-0.9.x or the 1.0.x branch.  I would prefer to relese this as 
1.1.0. (hopefully, that will happen sooner, rather than later).

Opinion from others?
Comment 15 Paul Querna 2005-03-14 06:35:39 UTC
This fix was released in 1.1.0