samba - Oct 2002 - File corruption with write cache enabled - patch included

I recently found out that write caching in samba sometimes leads to
file corruption (the setup program for Sophos Antivirus generates
corrupted files when making a "central installation" on a Samba
share).

This morning I tracked down the place in the Samba code that leads to
corruption. Here is what happened to me:

"write cache size" is 8192 bytes. A client opens a new file and writes
byte no. 30959. This byte is cached. Then the program write byte
no. 61919 which is written directly to the disk, since the cache
doesn't go that far. The client then writes bytes no. 0 through
61920. Since these bytes don't fit into the cache they are written
directly to the disk. However, the cached byte at position 30959 is
not discarded. When this byte is later written to the disk, the file
will get corrupted.

The patch below detects this situation and discards the cached
byte(s). I guess that some profiling code should also be added at some
time. The patch is against version 2.2.5.

Perhaps you should warn users of current versions against using write
caching.

I believe that this bug is the same as bug no. 24502 submitted by
Henrik Qwinto <qwinto@tut.by>.

Best regards, and thank you for making Samba,

Rasmus

-- 
Rasmus Borup Hansen, system adm.     Email: rbh@math.ku.dk
Institute for Mathematical Sciences  Phone: +45 353 20759
Universitetsparken 5                 Cell phone: +45 20829308
DK-2100 ?, Denmark                   Office: E208


--- samba-2.2.5/source/smbd/fileio.c.orig	Thu Oct  3 10:41:36 2002
+++ samba-2.2.5/source/smbd/fileio.c	Thu Oct  3 12:45:14 2002
@@ -438,7 +438,21 @@
 			if ( n <= wcp->alloc_size && n > wcp->data_size) {
 				cache_flush_needed = True;
 			} else {
+
 				ssize_t ret = real_write_file(fsp, data, pos, n);
+			  
+			        /*
+			         * If the write overlaps the entire
+			         * cache, then discard the cache.
+				 */
+
+			        if ( (pos <= wcp->offset) && 
+				     (pos + n >= wcp->offset + wcp->data_size) ) {
+					DEBUG(9,("discarding invalidated write cache: fd = %d, off=%.0f,
size=%u\n",
+						 fsp->fd, (double)wcp->offset, 
+						 (unsigned int)wcp->data_size ));
+					wcp->data_size = 0;
+				}
 
 				DO_PROFILE_INC(writecache_direct_writes);
 				if (ret == -1)

On Thu, Oct 03, 2002 at 12:52:10PM +0200, Rasmus Borup Hansen
wrote:
> I recently found out that write caching in samba sometimes leads to
> file corruption (the setup program for Sophos Antivirus generates
> corrupted files when making a "central installation" on a Samba
> share).
> 
> This morning I tracked down the place in the Samba code that leads to
> corruption. Here is what happened to me:
> 
> "write cache size" is 8192 bytes. A client opens a new file and
writes
> byte no. 30959. This byte is cached. Then the program write byte
> no. 61919 which is written directly to the disk, since the cache
> doesn't go that far. The client then writes bytes no. 0 through
> 61920. Since these bytes don't fit into the cache they are written
> directly to the disk. However, the cached byte at position 30959 is
> not discarded. When this byte is later written to the disk, the file
> will get corrupted.
> 
> The patch below detects this situation and discards the cached
> byte(s). I guess that some profiling code should also be added at some
> time. The patch is against version 2.2.5.
> 
> Perhaps you should warn users of current versions against using write
> caching.
> 
> I believe that this bug is the same as bug no. 24502 submitted by
> Henrik Qwinto <qwinto@tut.by>.
> 
> Best regards, and thank you for making Samba,
Damn good call ! Very good bugfix. I've committed it to all
Samba branches. Thanks a *lot* for this fix !

Jeremy.