Package: libapr1 Version: 1.4.6-3+deb7u1 Severity: grave Tags: patch, upstream
Hi, libapr1 uses fcntl(F_SETLKW) for locking files, but this is not compatible with multithreaded programs. fcntl(F_SETLKW) has the strange quirk that if an open and locked file is opened and then closed a second time in the same process, the lock is lost. This is something that may happen frequently in multithreaded programs, such as the apache2 mpm worker. fd1 = open("foo", O_RDWR|O_CREAT); fcntl(fd1, F_SETLKW, ...); /* file is now locked */ fd2 = open("foo", O_RDONLY); /* file is still locked */ close(fd2); /* file is no longer locked! */ ... Since file locking in libapr1 is broken^Wviolates the principle of least surprise, dataloss can very likely happen. I haven't checked the POSIX specs to see if this is expected behavior but I was able to reproduce it on both Linux and FreeBSD. A patch is attached that extends the libapr1 test suite to detect this situation. While libapr1 defaults to fcntl() locking it also supports flock(), which does not have the problems outlined above. A patch is attached which makes libapr1 use flock() even if fcntl() locking is available. We found this bug when investigating error messages from libapache2-mod-auth-cas that its session files were getting corrupted: [error] [client 127.0.0.1] MOD_AUTH_CAS: Error parsing XML content for '01234567890abcdef01234567890abcd' (Internal error), referer: https://www.example.com/ Switching to the flock() mechanism solved these problems. In other words, this bug is causing problems in real life, and is not just theoretical. This bug was found, reported to me and patched by Wessel Dankers. Thanks, Bye, Joost van Baal-Ilić -- Joost van Baal-Ilić http://abramowitz.uvt.nl/ Tilburg University The Netherlands
diff -ur apr-1.4.6,orig/file_io/unix/flock.c apr-1.4.6,fixed/file_io/unix/flock.c --- apr-1.4.6,orig/file_io/unix/flock.c 2006-08-03 12:55:31.000000000 +0200 +++ apr-1.4.6,fixed/file_io/unix/flock.c 2014-06-27 10:28:48.721611923 +0200 @@ -27,7 +27,7 @@ { int rc; -#if defined(HAVE_FCNTL_H) +#if defined(HAVE_FCNTL_H) && 0 { struct flock l = { 0 }; int fc;
diff -ur apr-1.4.6,orig/test/testflock.c apr-1.4.6,test/test/testflock.c --- apr-1.4.6,orig/test/testflock.c 2010-03-07 16:06:47.000000000 +0100 +++ apr-1.4.6,test/test/testflock.c 2014-06-27 10:18:59.786062499 +0200 @@ -60,6 +60,7 @@ static void test_withlock(abts_case *tc, void *data) { apr_file_t *file; + apr_file_t *file2; apr_status_t rv; int code; @@ -71,6 +72,12 @@ rv = apr_file_lock(file, APR_FLOCK_EXCLUSIVE); APR_ASSERT_SUCCESS(tc, "Could not lock the file.", rv); ABTS_PTR_NOTNULL(tc, file); + + /* open and close the file another time, to see if that messes with things */ + rv = apr_file_open(&file2, TESTFILE, APR_FOPEN_WRITE, APR_OS_DEFAULT, p); + APR_ASSERT_SUCCESS(tc, "Could not open file.", rv); + ABTS_PTR_NOTNULL(tc, file2); + (void) apr_file_close(file2); code = launch_reader(tc); ABTS_INT_EQUAL(tc, FAILED_READ, code);
signature.asc
Description: Digital signature