[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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

Your message dated Thu, 20 Aug 2015 12:04:33 +0000
with message-id <E1ZSOa9-0003FR-HM@franck.debian.org>
and subject line Bug#752872: fixed in apr 1.5.2-3
has caused the Debian Bug report #752872,
regarding libapr1: file locking is broken, leading to file corruption in e.g. libapache2-mod-auth-cas session files
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
752872: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=752872
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
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);

Attachment: signature.asc
Description: Digital signature


--- End Message ---
--- Begin Message --- 
Source: apr
Source-Version: 1.5.2-3

We believe that the bug you reported is fixed in the latest version of
apr, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 752872@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Stefan Fritsch <sf@debian.org> (supplier of updated apr package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Thu, 20 Aug 2015 13:47:59 +0200
Source: apr
Binary: libapr1 libapr1-dev libapr1-dbg
Architecture: source amd64
Version: 1.5.2-3
Distribution: unstable
Urgency: medium
Maintainer: Debian Apache Maintainers <debian-apache@lists.debian.org>
Changed-By: Stefan Fritsch <sf@debian.org>
Description:
 libapr1    - Apache Portable Runtime Library
 libapr1-dbg - Apache Portable Runtime Library - Debugging Symbols
 libapr1-dev - Apache Portable Runtime Library - Development Headers
Closes: 752872
Changes:
 apr (1.5.2-3) unstable; urgency=medium
 .
   * Use flock for apr_file_lock()/_unlock(). This has the advantage that
     the lock is per FD and not per process. On the other hand, locking over
     NFS may not work any more for non-Linux kernels. Closes: #752872
   * Add debug output for strange test failure on reproducible.debian.net.
Checksums-Sha1:
 fd2d4e008c45cb069653c98f5ed54d97efdeb2d3 2090 apr_1.5.2-3.dsc
 c1238f85359c0424123e23e32c2b25862e7a2b48 18572 apr_1.5.2-3.debian.tar.xz
 3c5be97ccb370c3d4f85b2e757ec773ecf1c9ede 259212 libapr1-dbg_1.5.2-3_amd64.deb
 06a43f385010ab33433d1e463bb0189a776f6764 665498 libapr1-dev_1.5.2-3_amd64.deb
 adc1938ee9bbd3a1358f6e4c8aeaeb7beb8080a4 96152 libapr1_1.5.2-3_amd64.deb
Checksums-Sha256:
 2344484f62544881344defbb1076ca6cce51f930f1a5abd359e65eafb0e169cb 2090 apr_1.5.2-3.dsc
 442ffb9a7225cf405fe7a2b4a4624543fcb93e6f14cccf22acd57916cfa8348d 18572 apr_1.5.2-3.debian.tar.xz
 39583dbc97880ae1e45c6fbf4ac95b00795ce7a3ae16842a054882159ce380de 259212 libapr1-dbg_1.5.2-3_amd64.deb
 8a2a48c9c4f725d31d7074f135b096736470a6e1177aeb25317a3a29f5686f62 665498 libapr1-dev_1.5.2-3_amd64.deb
 7ae30bd9dd718f15566a20b484aec67fb6e0ca704cb07f4f79d7b39b7414de18 96152 libapr1_1.5.2-3_amd64.deb
Files:
 98c97f7f43c1ad56e66e03cdef73284e 2090 libs optional apr_1.5.2-3.dsc
 9d0e01728178cd6eb9c92566003800b2 18572 libs optional apr_1.5.2-3.debian.tar.xz
 a6f1b81e674fed760195006b3104e119 259212 debug extra libapr1-dbg_1.5.2-3_amd64.deb
 d312390fb4e8d8bffd4e5172cd0fb028 665498 libdevel optional libapr1-dev_1.5.2-3_amd64.deb
 6be02240ae717aa478a96a1d84391fba 96152 libs optional libapr1_1.5.2-3_amd64.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIVAwUBVdW/osaHXzVBzv3gAQgAdBAAgRbNb/BDf6GwCFmOrAnhLHpRWoeRSC/1
uS/JdpgJfqNpAmCajKA5m/D+J1BcMq/Nfl0/Zy/flky4lBIIgSxMRxVw2QUyxNCm
zylBfdk2lTV2hXGzA7rZPSkpWt5cYUHABqBk2Wqdv+oogRTlYuyFKqgKEGjfcQSy
JU/RvCFu5PfY0k0oAf8khn197ButebrdPMNscu/K1hAIqFiojHXr1pGdpPT7pGqP
RE1AwkE1hL8GKisaYtmvIPmHeXXg+6sXkdbQN0xQskMp1untyF/wkaIpBWe393XM
PalDT1QjbrqV70QH45bYJma2U7bU1fWj+eZDhZ7e/1cLVO96VSB0F2w5z30U3p4e
Oj6oEPMw9s4rBWZjdQmIAVleKoClDzu/cgHEGDcCU/4c3JW/hpyCcZhBoTylfzni
fapqS0SxGlBovHJnxHNx5/GbvsiqwZ0xpy5/LYyWaL1D2LkM8mgLczI4AwUJIlwJ
A49+GJbPi5nvy6NEIYnfotnJ4bqB5+YSnkITg7x4Mx65UqNZ6TdSeYHO+yAUS1q7
XkVxN3TnRJst4e/fSdqwPkHTINIRT4PhwMLLdoVRPPKBueU+fjlaeYYG5Fs1xh1A
eg7QUar8O1F9WfFT6QALAsiqnp0Bzdup58jw4BrGAuztSPFg61K7Gnue4IMK7M76
7kOFs25usP4=
=K/1m
-----END PGP SIGNATURE-----

--- End Message ---
Reply to: