[PATCH] Fix for bug #9706 - Parameter is incorrect on Android

Mon Mar 18 13:38:01 MDT 2013

Hi Jeremy,

>> > Ok, I've done the tests. Looks like the correct response
>> > is to return shorter reads than what the client asks if
>> > the reads are out of range. The only case where we should
>> > still fail is when the read request out of range and is
>> > done in an andX chain, so I've left that code check in
>> > place.
>> > 
>> > I have a modified server patch that does this. It'll make
>> > Andrew happy as we no longer need to look at RA_SAMBA, as
>> > we just cope in the same way as Windows does with whatever
>> > the client asks for. All we do is modify max_pdu to cope
>> > with what the client told us it can cope with. Client tests
>> > also expanded to cope with signed+sealed requests as well
>> > as the new read values.
>> > 
>> > I'll post on the bug and a new patchset here shortly.
> And here is the fixed patchset. It applies cleanly
> to master.. I'll also upload a 4.0.next version
> to the bug report.
> 
> Please test and push if you're happy with it.

Sorry, I was really busy with other stuff...

> From a490a85376da04f73f50e036a6d1d3a969ac0367 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Wed, 13 Mar 2013 14:56:06 -0700
> Subject: [PATCH 01/10] Ensure we send CAP_LARGE_READX/CAP_LARGE_WRITEX to the
>  server on SMB1 sessionsetupX.
> 
> A Windows client doesn't do this, but that's because they never do large
> reads/writes over SMB1. It's in the spec that this should occur, and
> Samba 3.6.x uses it. Also, MacOSX, CIFSFS and Android (jCIFS) honour
> this setting and send it. We need to as well in order to give servers
> a better idea of our capabilities.

All tests have shown that it should make no difference if the client
send CAP_LARGE_READX or CAP_LARGE_WRITEX. Even if the client doesn't
send it, it's possible to do large reads and write.

If the client doesn't support large reads or write it would just doesn't
trigger them, there's really nothing more a client need to tell the server.

That's why we should should just skip this patch.

> From e40984781f28ff25a7f7aec0d00fbf6fdb7e5cbb Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Wed, 13 Mar 2013 15:23:52 -0700
> Subject: [PATCH 02/10] smb1cli_inbuf_parse_chain() and
> smb1cli_conn_dispatch_incoming() should use smb_len_tcp.

> They have to cope with large READX call replies that have
> a length greater than smb_len_nbt() can handle.

This one is fine.

> From 8796fa108100942089b5d29a3a856e69e2993248 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Fri, 15 Mar 2013 11:53:04 -0700
> Subject: [PATCH 03/10] Remove server_will_accept_large_read() and erroneous
> comment.
>
> We're going to replace this with a function that calculates
> the max PDU to return on a read and supports short reads.

This also looks good.

> Subject: [PATCH 07/10] Add set_XX() functions for fields we need to modify for
>  testing large READX.

Instead of fixing the connection values, we should just remove the silly
restriction
in cli_read_andx_create() that check for cli_read_max_bufsize(),
the server should just return a short read if wanted.

> From 094b76bc108da4cd001022d16cf2558fab208ea8 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Fri, 15 Mar 2013 11:57:48 -0700
> Subject: [PATCH 04/10] Add functions calc_max_read_pdu()/calc_read_size() to
>  work out the length we should return.
> 
> LARGE_READX test shows it's always safe to return a short read.
> Windows does so. Do the calculations to return what will fit
> in a read depending on what the client negotiated.
> 
> Conservative, in that if no flags are negotiated then the
> max read is set to default Win2K12 max_pdu size of 0x10000 + headers.
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>

(smb_size -4 + 12*2) seems to be wrong, which my modified version
I trigger the 0xffffff smb_panic in create_outbuf().

calc_max_read_pdu() should only calculate the max_pdu size and not the
0x10000 windows limitation, and it should not depend on
smb1.unix_info.client_cap_low nor on global_client_caps.

For the encrypted case we should use the max_send size from the client
instead.
The same applies to chained requests, which means we should not return a
short read
instead of an error for chained requests.

> From 69c288d13e76c7196c8c58dd375b2e1d6345f5fc Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Wed, 13 Mar 2013 15:43:21 -0700
> Subject: [PATCH 08/10] Add new LARGE_READX test to investigate large SMBreadX
> behavior.

calc_expected_return() should check for CIFS_UNIX_LARGE_READ_CAP instead of
any value in cli->server_posix_capabilities, we also need to
fill call cli_unix_extensions_version() if the server returns CAP_UNIX.

I've attached my current patches, but they're not complete
and fail make test. I'll try to fix this tomorrow.

Thanks for your patience!

metze
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tmp.diff
Type: text/x-diff
Size: 23733 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130318/ebd11007/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130318/ebd11007/attachment.pgp>