[Samba] The connection would be broken when read rate is too big

[Jeremy Allison]

On Wed, Mar 01, 2017 at 04:29:49PM -0800, Jeremy Allison wrote:
> On Wed, Mar 01, 2017 at 08:31:32AM +0000, Chenyehua via samba wrote:
> > Hi samba-team,
> > I met a problem recently:
> > 
> > Problem Description:
> > The connection would be broken when read rate is too big.
> > 
> > Environment:
> > Samba 4.3.11
> > Ubuntu server 14.04
> > 
> > Configuration:
> > usershare allow guests = yes
> >    max protocol = SMB3
> >    large readwrite = yes
> >    use sendfile = yes
> >    aio read size = 1024
> >    oplocks = yes
> >    deadtime = 10
> >    aio write behind = true
> >    clustering = yes
> >    store dos attributes = yes
> >    vfs objects = acl_xattr
> >    socket options = TCP_NODELAY SO_RCVBUF=131072 SO_SNDBUF=131072
> >    ctdbd socket = /var/run/ctdb/ctdbd.socket
> >    log level = 2
> >    security = user
> > 
> > I add some logs, found when the send queue is more than the max, it won’t allocate memory to the state->req in smbd_smb2_request_next_incoming function.
> > Then, the smbd_smb2_io_handler function mark the fd to not readable because state->req == NULL, and never mark it to readable again.
> > Then the Request Expiration Timer will break the connection.
> > 
> > I don’t know why the fd is marked to readable when the send queue is more than the max.
> > Or why mark the fd readable but not allocate memory to the state->req.
> > 
> > My solution:
> > Allocate memory to the state->req before return when the send queue is more than the max in smbd_smb2_request_next_incoming function.
> > 
> > Could you give us some help about the problem.
> 
> I'm looking at this now. Your report looks correct.
> Can you log a bug at bugzilla.samba.org so we can
> track this ?
> 
> I'm redirecting the reply to samba-technical at lists.samba.org
> as it more properly belongs there than at samba at lists.samba.org.

Can you try the following (raw and untested) patch ?

It should also apply to 4.3.11.

If you can confirm it fixes your problem then I'll
make sure it gets pushed upstream.

Thanks !

Jeremy.
-------------- next part --------------
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index b0a4afc8675..acaa0126dd8 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -3566,6 +3566,7 @@ static NTSTATUS smbd_smb2_flush_send_queue(struct smbXsrv_connection *xconn)
 	int ret;
 	int err;
 	bool retry;
+	NTSTATUS status;
 
 	if (xconn->smb2.send_queue == NULL) {
 		TEVENT_FD_NOT_WRITEABLE(xconn->transport.fde);
@@ -3577,11 +3578,12 @@ static NTSTATUS smbd_smb2_flush_send_queue(struct smbXsrv_connection *xconn)
 		bool ok;
 
 		if (e->sendfile_header != NULL) {
-			NTSTATUS status = NT_STATUS_INTERNAL_ERROR;
 			size_t size = 0;
 			size_t i = 0;
 			uint8_t *buf;
 
+			status = NT_STATUS_INTERNAL_ERROR;
+
 			for (i=0; i < e->count; i++) {
 				size += e->vector[i].iov_len;
 			}
@@ -3653,6 +3655,16 @@ static NTSTATUS smbd_smb2_flush_send_queue(struct smbXsrv_connection *xconn)
 		talloc_free(e->mem_ctx);
 	}
 
+	/*
+	 * Restart reads if we were blocked on
+	 * draining the send queue.
+	 */
+
+	status = smbd_smb2_request_next_incoming(xconn);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
 	return NT_STATUS_OK;
 }