Processed: Re: Bug#752872: libapr1: file locking is broken, leading to file corruption in e.g. libapache2-mod-auth-cas session files

2014-08-16 Thread Debian Bug Tracking System
Processing commands for cont...@bugs.debian.org:

 severity 752872 important
Bug #752872 [libapr1] libapr1: file locking is broken, leading to file 
corruption in e.g. libapache2-mod-auth-cas session files
Severity set to 'important' from 'grave'
 found 752872 1.4.6-3
Bug #752872 [libapr1] libapr1: file locking is broken, leading to file 
corruption in e.g. libapache2-mod-auth-cas session files
Marked as found in versions apr/1.4.6-3.
 thanks
Stopping processing here.

Please contact me if you need assistance.
-- 
752872: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=752872
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#752872: libapr1: file locking is broken, leading to file corruption in e.g. libapache2-mod-auth-cas session files

2014-08-16 Thread Stefan Fritsch
severity 752872 important
found 752872 1.4.6-3
thanks

On Friday 27 June 2014 11:37:18, Joost van Baal-Ilić wrote:
 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.

flock does not support locking on NFS (at least according to its man 
page), while fcntl does. I am undecided which is worse.

Since there does not seem to be a good solution, I am downgrading the 
severity of this bug.


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#752872: libapr1: file locking is broken, leading to file corruption in e.g. libapache2-mod-auth-cas session files

2014-06-27 Thread Joost van Baal-Ilić
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.0 +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.0 +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