DOS clients slow on Samba 4.1.7+ (bug 10422)

[Jeremy Allison]

On Fri, Oct 10, 2014 at 02:08:30PM +0100, Dominic Raferd wrote:
> Sorry I am not a Samba technical expert, but I believe there is a
> new and quite severe bug for Samba 4.1.7+ introduced by bug fix
> 10422. Although intended to cope with max xmit > 64kb it is having
> unintended negative consequences for legacy client machines using
> DOS.
> 
> This message appears hundreds of thousands of times in logs (even
> with log level 0), and file accesses are slow slow slow:
> 
>   reply_read: requested read size (1024) is greater than maximum
> allowed (972/1024). Returning short read of maximum allowed for
> compatibility with Windows 2000.
> 
> The problem seems to be that samba has max_send is 972 and the new
> bugfix code therefore blocks a *read* size of 1024. However the
> connections for these clients always worked perfectly before with
> the old test (which was against *max_recv* instead of max_send)
> (e.g. with Samba 4.1.5).
> 
> The protocol my DOS clients are using is what Samba calls 'LANMAN2'
> (Microsoft Networking).
> 
> I have tried different smb.conf settings (max xmit, socket options
> SO_SNDBUG and SO_RCVBUF) and also varied settings on the DOS
> clients, all to no avail. My conclusion is that Samba 4.1.7+ is
> broken, or at least badly wounded, for these clients.
> 
> I would be very grateful if it could be fixed!

Can you log a bug in buzilla and upload a wireshark
packet capture trace (including negprot onwards) to
the bug report please.

In the meantime, and after reading the MS-CIFS doc
I'm wondering if the server is over protecting
the client from its own folly here.

After all, SMB_COM_READ (and SMB_COM_LOCK_AND_READ) state:

CountOfBytesToRead (2 bytes): This field is a 16-bit unsigned integer indicating the number of
bytes to be read from the file. The client MUST ensure that the amount of data requested will fit in
the negotiated maximum buffer size.

So it is the client, not server responsibility
to send a correct size here.

Given that, you could try the following (untested)
patch...

Jeremy.
-------------- next part --------------
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 3f5b950..93675b4 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -3534,11 +3534,23 @@ void reply_lockread(struct smb_request *req)
 	maxtoread = sconn->smb1.sessions.max_send - (smb_size + 5*2 + 3);
 
 	if (numtoread > maxtoread) {
-		DEBUG(0,("reply_lockread: requested read size (%u) is greater than maximum allowed (%u/%u). \
-Returning short read of maximum allowed for compatibility with Windows 2000.\n",
+		/*
+		 * Just log the message but don't reduce the size.
+		 *
+		 * MS-CIFS states: SMB_COM_LOCK_AND_READ:
+		 * CountOfBytesToRead (2 bytes): This field is a
+		 * 16-bit unsigned integer indicating the number of
+		 * bytes to be read from the file. The client MUST
+		 * ensure that the amount of data requested will fit in
+		 * the negotiated maximum buffer size.
+		 *
+		 * So for old clients this isn't our problem. As
+		 * this command can never be chained (we check), this is safe.
+		 */
+
+		DEBUG(2,("reply_lockread: requested read size (%u) is greater than maximum allowed (%u/%u).\n",
 			(unsigned int)numtoread, (unsigned int)maxtoread,
 			(unsigned int)sconn->smb1.sessions.max_send));
-		numtoread = maxtoread;
 	}
 
 	reply_outbuf(req, 5, numtoread + 3);
@@ -3617,11 +3629,23 @@ void reply_read(struct smb_request *req)
 	maxtoread = sconn->smb1.sessions.max_send - (smb_size + 5*2 + 3);
 
 	if (numtoread > maxtoread) {
-		DEBUG(0,("reply_read: requested read size (%u) is greater than maximum allowed (%u/%u). \
-Returning short read of maximum allowed for compatibility with Windows 2000.\n",
+		/*
+		 * Just log the message but don't reduce the size.
+		 *
+		 * MS-CIFS states: SMB_COM_READ:
+		 * CountOfBytesToRead (2 bytes): This field is a
+		 * 16-bit unsigned integer indicating the number of
+		 * bytes to be read from the file. The client MUST
+		 * ensure that the amount of data requested will fit in
+		 * the negotiated maximum buffer size.
+		 *
+		 * So for old clients this isn't our problem. As
+		 * this command can never be chained (we check), this is safe.
+		 */
+
+		DEBUG(2,("reply_read: requested read size (%u) is greater than maximum allowed (%u/%u).\n",
 			(unsigned int)numtoread, (unsigned int)maxtoread,
 			(unsigned int)sconn->smb1.sessions.max_send));
-		numtoread = maxtoread;
 	}
 
 	reply_outbuf(req, 5, numtoread+3);