Bug 10406 - vfs_dirsort can trample on its own private data.
vfs_dirsort can trample on its own private data.
Status: CLOSED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-01-30 00:08 UTC by Jeremy Allison
Modified: 2014-10-13 15:50 UTC (History)
0 users

See Also:


Attachments
git-am fix for master. (7.39 KB, patch)
2014-01-30 01:11 UTC, Jeremy Allison
no flags Details
git-am backport of the fix that went into master. (9.53 KB, patch)
2014-02-12 19:30 UTC, Jeremy Allison
abartlet: review+
Details
git-am fix for 4.1.next (9.56 KB, patch)
2014-02-12 19:32 UTC, Jeremy Allison
abartlet: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2014-01-30 00:08:44 UTC
Reported by an OEM.

----------------------------------------------------------------
We use dirsort module in our product, and encounter some problems with
this module.
Since dirsort module only keeps one private data (i.e., struct
dirsort_privates) for per vfs_handle_struct.
Once the samba process calls SMB_VFS_OPENDIR multiple times, the previous data
will be overwritten, like the following example:

dirp1 = SMB_VFS_OPENDIR(dir_a);
dirp2 = SMB_VFS_OPENDIR(dir_b);
...
SMB_VFS_READDIR(dirp1); // this will get wrong results since the data was
overwritten in the second SMB_VFS_OPENDIR()
SMB_VFS_READDIR(dirp2);
...
SMB_VFS_CLOSEDIR(dirp2);
SMB_VFS_CLOSEDIR(dirp1);

According to our experiences, this scenario sometimes happens when users use 7-
zip to compress a big direcotry (i.e., a directory which contains many sub
folders and sub-sub folders).
Since dirsort module returns a wrong file list, users will get incorrect
archive files.
We think the private data should be stored for per dir pointer so that it will
not be overwritten by the other SMB_VFS_OPENDIR().

----------------------------------------------------------------

I have a fix for this to follow.
Comment 1 Jeremy Allison 2014-01-30 01:11:21 UTC
Created attachment 9621 [details]
git-am fix for master.

Current patch sent to the OEM for testing.
Comment 2 Jeremy Allison 2014-02-11 05:24:29 UTC
Response from OEM:

Yes, we have tested the patch and it can work correctly now.
Thanks for your help !
Comment 3 Jeremy Allison 2014-02-12 19:30:40 UTC
Created attachment 9678 [details]
git-am backport of the fix that went into master.

Minor changes around NTSTATUS inside opendir.
Comment 4 Jeremy Allison 2014-02-12 19:32:16 UTC
Created attachment 9679 [details]
git-am fix for 4.1.next

Cherry-picked cleanly from master.

Andrew please +1 on these two and I'll reassign to Karolin for inclusion in 4.1.next, 4.0.next.

Jeremy.
Comment 5 Jeremy Allison 2014-02-13 21:56:08 UTC
Thanks ! Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next.

Jeremy.
Comment 6 Karolin Seeger 2014-02-14 19:31:44 UTC
(In reply to comment #5)
> Thanks ! Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next.
> 
> Jeremy.

Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Comment 7 Karolin Seeger 2014-02-16 16:11:58 UTC
Pushed to v4-1-test and v4-0-test.
Closing out bug report.

Thanks!