[PATCH] smbclient fails with posix large reads

[Jeremy Allison]

On Sat, Jan 28, 2012 at 11:27:37AM +1100, Andrew Bartlett wrote:
> On Fri, 2012-01-27 at 16:07 -0800, Jeremy Allison wrote:
> > On Sat, Jan 28, 2012 at 08:50:12AM +1100, Andrew Bartlett wrote:
> > > On Fri, 2012-01-27 at 13:39 -0800, Jeremy Allison wrote:
> > > > On Fri, Jan 27, 2012 at 11:56:13AM +1100, Andrew Bartlett wrote:
> > > > > smbclient3 in master fails when we attempt to use the posix extensions
> > > > > to read 16MB chucks at a time.  Attached is a fix, which also applies to
> > > > > (at least) 3.5.
> > > > > 
> > > > > to reproduce:
> > > > > 
> > > > > smbclient //server/tmp
> > > > > 
> > > > > posix
> > > > > get large_file
> > > > > 
> > > > > I've CC'd Alex who found the issue.
> > > > > 
> > > > > The simple issue is that smb_buffer_oob fails because it is told the
> > > > > incorrect packet size, not taking into account that the packet may be a
> > > > > large read&X.  There are lot of calls to smb_len() in the code - I
> > > > > suspect this may not be the only one wrong...
> > > > > 
> > > > > Given this was for an OOB check, I wanted to run it past the list to
> > > > > double-check it. 
> > > > 
> > > > LGTM - I'm pushing it to master (and the associated test).
> > > > 
> > > > I'll raise a bug to get it applied to 3.6.x also.
> > > 
> > > For 3.5 the macro name changed, here is the patch.
> > 
> > Thanks ! Fixes uploaded to:
> > 
> > https://bugzilla.samba.org/show_bug.cgi?id=8727
> > 
> > for you to review for 3.5.next and 3.6.next.
> 
> The next task in this saga will be to review all the other smb_len
> calls.  While I've checked the particular interaction between this call
> and the original packet read, I worry about the rest.
> 
> If we were ever to read a packet with smb_len(), then check it's
> 'length' with smb_len_tcp(), we could act on the extra bits that were
> unchecked at socket read time. 
> 
> For this reason, I think we should always use smb_len_tcp().  If we have
> not negotiated large reads, or this particular packet should not be so
> large, we can then discard an invalid packet using the high bits based
> on a simple if (len > MAX_NBT_LEN) or such.  
> 
> Additional care should be taken around the encrypted cifs packets, as
> these appear to use smb_len() currently.  

I think we only use the 16mb size when it's a readX reply, which
is why I believe your current patch is correct.

Won't hurt to check though. :-).

Jeremy.