Bug 5517 - Make Test Failure for RW1
Summary: Make Test Failure for RW1
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: smbclient (show other bugs)
Version: 3.0.30
Hardware: Sparc Solaris
: P3 major
Target Milestone: none
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-03 14:29 UTC by David Eisner
Modified: 2008-06-03 15:12 UTC (History)
0 users

See Also:


Attachments
Patch (908 bytes, patch)
2008-06-03 15:12 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Eisner 2008-06-03 14:29:00 UTC
I'm trying to build and install Samba 3.0.30 on a Solaris 9 SPARC machine.

When I do a "make test", the RW1 test is failing.   If I go back and
configure and build 3.0.28 with the same settings, and do a make test,
everything passes.  Here's what I'm seeing with 3.0.30:

---8<---
Testing RW1 (0)
TEST OUTPUT:
host=127.0.0.2 share=tmp user=root myname=cannes
Running RW1
starting readwritetest
unlink failed (NT_STATUS_OBJECT_NAME_NOT_FOUND) (normal, this file
should not exist)
Passed readwritetest v1: Yes
unlink failed (NT_STATUS_OBJECT_NAME_NOT_FOUND) (normal, this file
should not exist)
read failed (Read error: Error 0)
read -1, expected 130534
close failed (Read error: Error 0)
close failed (Read error: Error 0)
unlink failed (Read error: Error 0)
Passed readwritetest v2: No
tdis failed (Read error: Error 0)
TEST RW1 FAILED!
RW1 took 18.458818 secs

TEST FAILED: /export/data/software/cradle/build/samba-3.0.30/source/bin/smbtorture
//127.0.0.2/tmp -Uroot%test RW1 (status 1)
--->8---

I did some more research and found this (from my message to the samba list, more or less):

---8<---
Hmm, I wonder if this isn't a bug with the fix for CVE-2008-1105.

Here's what seems to be going on at a low level:

When the client state is setup in run_readwritetest() by way of
torture_open_connection(), cli_state->bufsize gets set to
CLI_SAMBA_MAX_LARGE_READX_SIZE, which is defined to be 127*1024 ==
130048.

During the rw_torture2 iteration that breaks the test,
send_file_readX() in smbd/reply.c calculates the packet length to
send:

   nread = read_file(fsp,data,startpos,smb_maxcnt);
   // ...

   outsize = set_message(outbuf,12,nread,False);
   // ...

   /* Returning the number of bytes we want to send back - including header. */
   return outsize;
}

When RW1 fails, nread == 130048 (which is
CLI_SAMBA_MAX_LARGE_READX_SIZE), and outsize is set by set_message()
to be 39 + 2x12 + nread == 130111.

Later on, construct_reply reduces this by 4, and I think this becomes
the length of the reply packet:

 static int construct_reply(char *inbuf,char *outbuf,int size,int bufsize)
 {
     // ...
     if(outsize > 4)
         smb_setlen(outbuf,outsize - 4);
     return(outsize);
 }

Now the length stored in the packet is 130111 - 4 == 130107

Back on the client side, receive_smb_raw()  (in lib/util_sock.c) is unhappy:

 BOOL receive_smb_raw(int fd, char *buffer, size_t buflen, unsigned
int timeout)
 {
     // ...
     len = read_smb_length_return_keepalive(fd,buffer,timeout);
     // ...
     if (len > buflen) {
         DEBUG(0,("Invalid packet length! (%lu bytes).\n",(unsigned long)len));
     //...

And here's the output (I added a debugging statement to also print buflen:

##DRE: receive_smb_raw: Invalid packet length! len == (130107 bytes),
buflen == (130048).

That is, it's complaining because 130107, the length reported in the
reply packet header (I think), is larger than bufsize for the client
state.  As to where the real problem is (i.e. does receive_smb_raw()
need to do something with len before comparing it with buflen), I
can't say.
--->8---

And then Jeremy Allison responded with this message and a patch (which I believe has already been committed: 

---8<---
Ah, I see the problem. CLI_SAMBA_MAX_LARGE_READX_SIZE is 127k
on the client, the server buffer size is 128k. For client large
readx/writex I should be allocating CLI_SAMBA_MAX_LARGE_READX_SIZE
+ LARGE_WRITEX_HDR_SIZE + SAFETY_MARGIN, not just
CLI_SAMBA_MAX_LARGE_READX_SIZE + SAFETY_MARGIN. It's safe as the
"safety margin" protects us but the client buffer detection
complains.

Try this patch against 3.0.x - should fix the problem.

--->8---

I was asked to report the bug for reference, so here you go.
Comment 1 Jeremy Allison 2008-06-03 15:12:01 UTC
Created attachment 3334 [details]
Patch

Fix for 3.0.x and 3.2.x/3.3.x.
Jeremy.
Comment 2 Jeremy Allison 2008-06-03 15:12:30 UTC
Fixed in all git trees.
Jeremy.