Bug 5990 - strict allocate should be checked before ftruncate
Summary: strict allocate should be checked before ftruncate
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.2
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.2.6
Hardware: All Solaris
: P3 normal
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-24 02:48 UTC by Yasuma Takeda
Modified: 2008-12-25 14:14 UTC (History)
0 users

See Also:


Attachments
a patch for samba 3.2.6 (942 bytes, patch)
2008-12-24 02:50 UTC, Yasuma Takeda
no flags Details
Replacement patch (885 bytes, patch)
2008-12-24 17:18 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yasuma Takeda 2008-12-24 02:48:27 UTC
I investigated about copying a file which exceeds the disk size from WindowsXP to Samba3.2.6. It causes "NT_STATUS_DISK_FULL" error but its response take more time than Windows Server.
Because ftruncate is executed in that sequence.

I compared the behaviour of "NT_STATUS_DISK_FULL" between Samba and Windows 2003 Server by wireshark.

As a result, I found that SMB_TRANS2(SMB_SET_FILE_END_OF_FILE_INFO) should return "NT_STATUS_DISK_FULL" error.
So, I think vfs_set_filelen() should return a error before doing ftruncate when "strict allocate" is  "yes".
I made a patch for samba 3.2.6.

BTW, the ftruncate of Solaris10's ZFS seems to need more time than Ext3 of Linux.
So, if you try to check this issue on Linux, you might think there is no problem.

Thanks
Comment 1 Yasuma Takeda 2008-12-24 02:50:27 UTC
Created attachment 3826 [details]
a patch for samba 3.2.6
Comment 2 Jeremy Allison 2008-12-24 17:18:20 UTC
Created attachment 3828 [details]
Replacement patch

I think this patch (almost the same as yours) is a better fit. Firstly, it moves the code to within strict_allocate_ftruncate() which copes with all the other logic needed when strict allocate is set, and secondly the size to compare from the dfree return needs to be space_to_write, which is the difference between the current length of the file and the new ftruncate extended length, not len, which is the new ftruncated length. If we compare against len then we're expecting the full size of the file to be available even if we're extending only by one byte.
Can you check this and confirm it fixes the problem ? If so I'll commit to all branches.

Thanks a *lot* - great catch of a problem in the code.

Jeremy
Comment 3 Yasuma Takeda 2008-12-24 23:08:39 UTC
I confirmed your patch on CentOS5(Ext3) and Solaris10(ZFS).
There is no problem.

Thanks for your work.
Comment 4 Jeremy Allison 2008-12-25 14:14:51 UTC
Applied to all branches, thanks !
Jeremy.