Bug 7188 - Logic error in check of total_data for call_trans2mkdir()
Summary: Logic error in check of total_data for call_trans2mkdir()
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.5.0rc3
Hardware: All All
: P3 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-28 17:04 UTC by Sprow
Modified: 2011-08-27 09:12 UTC (History)
0 users

See Also:

Attachments
Patch for 3.5.x. (2.01 KB, patch)
2010-03-12 18:32 UTC, Jeremy Allison 		metze: review+
Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sprow 2010-02-28 17:04:28 UTC 
This trans2 subcommand (0x000D = CREATE_DIRECTORY) can contain optional EA data.
Back in 3.0.20b the logic was changed from

 if (total_data) {
       do something with it
 }

to fix bug #3212. However, the fix now means that a total_data of zero results in smbd returning "bad parameter" error to the client.

To paraphrase 3.5.0rc3, or indeed anything post 3.0.20b, it now does

 if (total_data != 4) {
       if (total_data < 10) return error;
       do something with it
 } else {
       ignore it
 }
 
in other words a total_data of 0 is now faulted, which is wrong.

I think that just inverting the 'if' to say

 if ((total_data == 0) || (total_data == 4)) {
       ignore it
 } else {
       if (total_data < 10) return error;
       do something with it
 }

would fix this.
Comment 1 Jeremy Allison 2010-03-12 17:41:49 UTC 
What client are you using that can reproduce this ? I'm interested to know.

Jeremy.
Comment 2 Jeremy Allison 2010-03-12 18:32:31 UTC 
Created attachment 5495 [details]
Patch for 3.5.x.

Treats total_data consistently across trans2mkdir and trans2open - ignore total_data contents if total_data == 4, or 0. Make error processing regular.

Please test and let me know if it fixes your issue.

Jeremy.
Comment 3 Sprow 2010-03-13 07:40:30 UTC 
(In reply to comment #1)
> What client are you using that can reproduce this ? I'm interested to know.
> Jeremy.

The client is a BBC Master Microcomputer with ethernet upgrade - somewhat niche admittedly, the Trans2 packets are however correctly formed with a total_data of zero.

The server reported as rejecting this was the smbd in Mac OS X Snow Leopard (3.0.25b derivative), whereas Mac OS X Tiger had previously worked fine.
Comment 4 Sprow 2010-03-13 14:55:19 UTC 
(In reply to comment #2)
> Treats total_data consistently across trans2mkdir and trans2open - ignore
> total_data contents if total_data == 4, or 0. 

That patch fixes the problem - thanks!

> Make error processing regular.

I hadn't spotted that optimisation, nice.
Comment 5 Jeremy Allison 2010-03-15 17:06:49 UTC 
Comment on attachment 5495 [details]
Patch for 3.5.x.

If you think this is worth it, re-assign to Karolin for 3.5.x. Otherwise it's in master and will be in 3.6.0.

Jeremy.
Comment 6 Stefan Metzmacher 2010-05-20 03:25:20 UTC 
Comment on attachment 5495 [details]
Patch for 3.5.x.

Karolin, please pick
704a607e3c3a5c3e727b386fab
from master
Comment 7 Karolin Seeger 2010-05-20 05:32:43 UTC 
Pushed, will be included in 3.5.4.
Closing out bug report.

Thanks!
Comment 8 Karolin Seeger 2010-05-20 05:33:29 UTC 
Really closing out bug report now... ;-)