Padding byte in cifs readx response

[Christof Schmitt]

On Fri, Aug 15, 2014 at 11:18:20AM -0700, Jeremy Allison wrote:
> On Fri, Aug 15, 2014 at 07:27:21PM +0200, Volker Lendecke wrote:
> > On Fri, Aug 15, 2014 at 10:24:22AM -0700, Jeremy Allison wrote:
> > > On Fri, Aug 15, 2014 at 07:22:16PM +0200, Volker Lendecke wrote:
> > > > On Fri, Aug 15, 2014 at 10:17:15AM -0700, Jeremy Allison wrote:
> > > > > On Thu, Aug 14, 2014 at 10:30:52PM -0700, Christof Schmitt wrote:
> > > > > > On Wed, Aug 13, 2014 at 04:10:34PM +0200, Volker Lendecke wrote:
> > > > > > > We just overflowed the 16MB nbss packet. I've attached one
> > > > > > > question, a few R-Bs and a possible fix.
> > > > > > 
> > > > > > Thanks. The fix looks good, i included it in the patch series, see
> > > > > > attachment.
> > > > > > > 
> > > > > > > VL: I think at least in the aio case the padding byte is left uninitialized.
> > > > > > > Can you check that? Thanks!
> > > > > > 
> > > > > > Yes, i missed that. struct aio_extra is allocated and zeroed, but not
> > > > > > the following data buffer. I added the explicit initialization of the
> > > > > > padding byte.
> > > > > 
> > > > > LGTM Christof thanks ! Pushed.
> > > > 
> > > > Wait please!
> > > > 
> > > > I'm still dubious about the new_size += 1 in
> > > > smb_splice_chain. Can you explain that? I'm really nervous
> > > > about that piece of the code, &x is really from hell.
> > > 
> > > Ok, will hold off on that :-).
> > > 
> > > andX is indeed from the darkest pits of hades. But
> > > I thought you'd looked at that already - my mistake,
> > > sorry !
> > 
> > I would have thought that read&x already has that byte in
> > smb_buf, so that new_size will automatically have it. The
> > offset calculation -- sure. But the size increment?
> 
> Oh, that's a really good point. new_size is only
> used for talloc_realloc the outbuf buffer, not
> for setting any sizes within - so if it's off
> by one all further chain entries will creep
> forward by one also...
> 
> And chain_padding can only be up to 4 bytes,
> so after 4 chained readX requests we'd be
> screwed. Not an easy thing to test !

I took another look. The reason for changing the new_size was that it
only accounted for the data without the padding byte, but the bytes
pointer pointed to the padding byte. On a closer look this is not
entirely correct, since num_bytes is also used later for the byte count
field. Adjusting new_size instead might be a better approach. I will
take another look at this area.

Christof