Bug 2662 - revamped directory handling code is not 64bit clean
Summary: revamped directory handling code is not 64bit clean
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.0.3
Hardware: SGI IRIX
: P3 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 9082 9772 10341 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-04-27 21:08 UTC by James Peach
Modified: 2014-03-17 20:34 UTC (History)
8 users (show)

See Also:


Attachments
use SMB_OFF_T for directory offsets (15.65 KB, patch)
2005-04-27 21:21 UTC, James Peach
no flags Details
use SMB_OFF_T for directory offsets (15.94 KB, patch)
2005-04-28 16:02 UTC, James Peach
no flags Details
git-am fix for master. (14.17 KB, patch)
2014-01-13 19:09 UTC, Jeremy Allison
no flags Details
git-am fix for master. (14.18 KB, patch)
2014-01-13 19:12 UTC, Jeremy Allison
jra: review? (ddiss)
asn: review+
Details
git-am fix for 4.1.x (15.22 KB, application/octet-stream)
2014-01-15 21:21 UTC, Jeremy Allison
asn: review+
Details
git-am fix for 4.0.next (15.21 KB, patch)
2014-01-15 21:22 UTC, Jeremy Allison
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description James Peach 2005-04-27 21:08:57 UTC
On IRIX, sizeof(off_t) != sizeof(long), so the Jeremy's new directory
handling code that casts off_t to long truncates directory entry
offsets from 64 bits to 32 bits. There may be other systems for which 
the same is true.
Comment 1 James Peach 2005-04-27 21:21:26 UTC
Created attachment 1185 [details]
use SMB_OFF_T for directory offsets

This patch does 3 things
    1. uses SMB_OFF_T for seekdir/telldir instead of long
    2. defines _SGI_SOURCE in CPPFLAGS
    3. lazily allocates the dptr name cache

(1) is incomplete because dtrp_fetch and dptr_fill are not handled properly.
That is, the directory offsets are truncated before they are placed onto the
wire. I'll ping jra offline to ask his advice about this.

(2) lets us get a proper declaration of telldir on IRIX.  It's necessary
for historical reasons.

(3) just seemed like a nice thing to do :)
Comment 2 Jeremy Allison 2005-04-28 07:43:17 UTC
The Single UNIX Spec here :

http://www.opengroup.org/onlinepubs/009695399/functions/telldir.html

defines it as "long". But Linux (and I guess IRIX too) uses off_t.
How about I redefine it as SMB_OFF_T which is 64-bits on a 64-bit filesystem ?

Any conversion issues you can see with the current code (you need to look at
smbd/trans2.c also as the telldir offsets leak out there) ?

I'm on a plane to Germany today, won't get to this until tomorrow at
the earliest.

Jeremy.
Comment 3 James Peach 2005-04-28 16:02:13 UTC
Created attachment 1196 [details]
use SMB_OFF_T for directory offsets
Comment 4 James Peach 2005-04-28 16:09:03 UTC
(In reply to comment #2)
> The Single UNIX Spec here :
>
> http://www.opengroup.org/onlinepubs/009695399/functions/telldir.html
> 
> defines it as "long". But Linux (and I guess IRIX too) uses off_t.

With IRIX, you get to choose your declaration. The definition uses off_t
though, so you basically have to choose the right one :)

> How about I redefine it as SMB_OFF_T which is 64-bits on a 64-bit filesystem ?

That's what may patch does ..

> Any conversion issues you can see with the current code (you need to look at
> smbd/trans2.c also as the telldir offsets leak out there) ?

Did you check something in? If so, I'm not seeing it ..

I'm most interested in reply.c where dptr_fill and dptr_fetch are called. My
patch doesn't fix these, but putting 5 of a potential 8 bytes on the wire
makes me feel icky.


Comment 5 James Peach 2005-06-07 00:48:02 UTC
Any comments on this patch?
Comment 6 James Peach 2005-07-30 22:46:15 UTC
Reassign IRIX bugs to me.
Comment 7 Gerald (Jerry) Carter (dead mail address) 2005-09-27 13:41:51 UTC
what's teh status of this now?  Looking at ther current SAMBA_3_0...
Comment 8 Jeremy Allison 2005-09-27 14:05:28 UTC
It's still not 64-bit clean in the old client handling. We'd have to on-the-fly
create a mapping table from 32-bit replies to 64-bit offsets. I'm not going to
do this until someone complains :-). Remember it's only for Lanman1.x and below
clients :-).
Jeremy.
Comment 9 Gerald (Jerry) Carter (dead mail address) 2006-04-08 07:43:43 UTC
Cleaning up versions.  There was no 3.0.15 so leaving it in bugzilla 
is causing some confusion.  Moving these nuder 3.0.20.
Originally files against 3.0.15preX.
Comment 10 Sumit Bose 2012-08-09 12:12:55 UTC
This issue also can be found on Linux with DOS clients if the offset has more than 32bits, see https://bugzilla.redhat.com/show_bug.cgi?id=843765 .

According to MS-CIFS 2.2.4.58.1 the server state in the resume key has 16 bytes, but only the last 5 are used. Would it be possible to just use the last 9 bytes so that a 64bit offset can be stored here or are the other bytes used somewhere else?
Comment 11 Andreas Schneider 2012-09-11 12:27:50 UTC
*** Bug 9082 has been marked as a duplicate of this bug. ***
Comment 12 Andreas Schneider 2013-02-18 09:42:26 UTC
This popped up in RHEL6 with a kernel update and ext4 returning 64bit cookies now. Someone reported it in Fedora 18 too. The question is if we still support DOS clients.

NOTE: Turning off dir_index on the ext4 filesystem may work around the problem.
Comment 13 Conrad Kostecki 2013-04-08 20:49:41 UTC
I've also encountered this problem today.
Disabling dir_index on the ext4 filesystem worked for me.
Now, I can access a share again from DOS.

Well, I would be happy, if Samba would still support dos clients :)
Comment 14 Conrad Kostecki 2013-04-08 20:50:07 UTC
*** Bug 9772 has been marked as a duplicate of this bug. ***
Comment 15 Guenter Kukkukk 2014-01-10 03:07:25 UTC
*** Bug 10341 has been marked as a duplicate of this bug. ***
Comment 16 Jeremy Allison 2014-01-13 16:46:19 UTC
I have a fix for this in test right now (finally). Sorry for the delay..

Jeremy.
Comment 17 Jeremy Allison 2014-01-13 19:09:21 UTC
Created attachment 9586 [details]
git-am fix for master.

Adds a memcache entry to 64-bit old-searches, maps 64-bit directory offset cookies into 32-bit wire values.

Fixes the problem here. Please test !

Jeremy.
Comment 18 Jeremy Allison 2014-01-13 19:12:13 UTC
Created attachment 9587 [details]
git-am fix for master.

Fix error in comment describing new memcache type (it's not a talloc type).

Jeremy.
Comment 19 Jeremy Allison 2014-01-13 19:12:55 UTC
Comment on attachment 9587 [details]
git-am fix for master.

Andreas, David - I think this patch will fix the customer issues with old DOS clients against modern 64-bit servers. Please review !

Thanks,

Jeremy.
Comment 20 Guenter Kukkukk 2014-01-15 03:55:51 UTC
i've applied this patch to recent git master
and built both 32Bit and 64Bit versions.

Used a MSDOS client VM and _both_ samba versions
work as expected now! :-)

Cheers, Günter
Comment 21 Andreas Schneider 2014-01-15 08:49:21 UTC
Comment on attachment 9587 [details]
git-am fix for master.

Patchset is in autobuild.
Comment 22 Jeremy Allison 2014-01-15 21:21:38 UTC
Created attachment 9596 [details]
git-am fix for 4.1.x

What went into master with cherry-pick info.
Comment 23 Jeremy Allison 2014-01-15 21:22:19 UTC
Created attachment 9597 [details]
git-am fix for 4.0.next

What went into master (back ported for 4.0.next with cherry-pick info).
Comment 24 Andreas Schneider 2014-01-16 10:47:39 UTC
Comment on attachment 9596 [details]
git-am fix for 4.1.x

LGTM
Comment 25 Andreas Schneider 2014-01-16 10:48:34 UTC
Karolin, please add the patches to the next 4.x releases, thanks!
Comment 26 Karolin Seeger 2014-01-17 08:19:00 UTC
Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Comment 27 Karolin Seeger 2014-01-17 10:01:18 UTC
(In reply to comment #26)
> Pushed to autobuild-v4-1-test and autobuild-v4-0-test.

autobuild-v4-0-test fails after applying these patches (tried 2 times):

UNEXPECTED(error): samba3.base.defer_open.defer_open(s3dc)
REASON: _StringException: _StringException: Unknown error/failure. Missing torture_fail() or torture_assert_*() call?

Re-assigning to Jeremy.
Comment 28 Jeremy Allison 2014-01-17 22:33:33 UTC
Ok, doing:

~/src/samba/git/samba-4-0-test$ make test TESTS=samba3.base.defer_open

worked.

ALL OK (2 tests in 2 testsuites)


I'll try a full make test on the v-4-0-test code with these patches applied.

Jeremy.
Comment 29 Jeremy Allison 2014-01-22 00:07:16 UTC
Ok, doing a full make test on 4.0.x with these patches succeeded for me. It might be a flakey test. Can you try again ?

Jeremy.
Comment 30 Karolin Seeger 2014-01-25 15:23:39 UTC
(In reply to comment #29)
> Ok, doing a full make test on 4.0.x with these patches succeeded for me. It
> might be a flakey test. Can you try again ?
> 
> Jeremy.

Sure. Pushed again. Thanks for investigating!
Comment 31 Karolin Seeger 2014-02-05 10:18:17 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 32 Antti 2014-03-17 07:17:37 UTC
I had stumpled to this issue on Scientific Linux server with Samba 3.6.9 + DOS -client.

I tried if I could backport the fix but got some problem. DOS client works ok most of the time, but sometimes DIR command returns only the first entry [.].
Based on the log files, it seems that the empty directory listing is returned when dirptr=8, 40, 72 or 104.

I have very limited idea about the internals of the smbd and SMB protocol itself and cannot found the reason for that. Could someone with better skills check this?
Comment 33 Jeremy Allison 2014-03-17 17:52:27 UTC
We're not planning to release a new 3.6.x with this fix. Is there any chance you can update your Samba to a 4.0 or above release that contains this fix ?
Comment 34 Antti 2014-03-17 20:34:18 UTC
Well, actually I tried samba 4.0 with virtual machines running server and client and got the same issue. I dont't know if I messed something. Could someone else test the same? I'm very sorry if it's my fault, but I don't understand what could be the reason for this.

I simply connected DOS client to 64 bit server exporting EXT4 share, and typed dir command several times in directory containing some files. First 6 times it works ok, 7th command fails (dirptr=8), etc.

I'm not sure the final solution for my own situation now. I'm running the actual production server in our company and prefer RPM -packages, but this issue creates problem with the standard RH -based distribution (because of the shipped samba/kernel versions). Disabling DIR_INDEX helps, but I'm trying to decide the final long term workaround. I wouldn't like to test samba4 in production server before I'm sure it works flawless in all cases?