On Wed, Aug 6, 2014 at 10:21 AM, Pavel Shilovsky <pshilovsky@xxxxxxxxx> wrote:
> 2014-08-06 17:31 GMT+04:00 Jeff Layton <jlayton@xxxxxxxxx>:
>> On Wed, 6 Aug 2014 16:38:32 +0400
>> Pavel Shilovsky <pshilovsky@xxxxxxxxx> wrote:
>>> If server reuses a uniqueid - it is a bug in the server.
>>>
>>
>> Then I think that's probably another samba bug. IIRC, it generates
>> uniqueids using the st_ino and st_dev fields from stat(). There are
>> many Linux filesystems that will aggressively reuse inode numbers and
>> there is no API to get at the inode->i_generation field from userland.
>>
>> --------------[snip]----------------
>> /********************************************************************
>> Create a 64 bit FileIndex. If the file is on the same device as
>> the root of the share, just return the 64-bit inode. If it isn't,
>> mangle as we used to do.
>> ********************************************************************/
>>
>> uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf)
>> {
>> uint64_t file_index;
>> if (conn->base_share_dev == psbuf->st_ex_dev) {
>> return (uint64_t)psbuf->st_ex_ino;
>> }
>> file_index = ((psbuf->st_ex_ino) & UINT32_MAX); /* FileIndexLow */
>> file_index |= ((uint64_t)((psbuf->st_ex_dev) & UINT32_MAX)) << 32; /* FileIndexHigh */
>> return file_index;
>> }
>> --------------[snip]----------------
>>
>>> The creation time can be changed on the Windows server:
>>> 1) we call stat FILE - construct the inode with the recent info;
>>> 2) on the server another client calls SetFileTime() and updates
>>> CreationTime of the FILE;
>>> 3) we call stat FILE again, get the recent info but don't store the
>>> new CreationTime value and stay with the wrong CreationTime value.
>>>
>>> Also, in this case we can't rely on CreationTime to determine if this
>>> is the same inode or not. I think the best thing is to *trust* the
>>> server that uniqueid is unique (at least for SMB 2.0 and higher
>>> versions).
>>>
>>> Note, that now we use CreationTIme value for the inode search in
>>> iget(). The above example of changing this value on the server shows
>>> that we can end up with several inodes for each "creationtime" version
>>> of the file.
>>>
>>
>> Sure, it's settable and you can abuse it, but who actually does that?
>>
>> It's very rare for someone to mess with the create time like that. I
>> imagine it's generally only done by backup utilities when restoring
>> and maybe rootkits. Those are both sort of outside the scope of
>> normal usage.
>
> Microsoft Office Excel is doing this -- see
> http://www.techrepublic.com/blog/the-enterprise-cloud/why-writing-a-windows-compatible-file-server-is-still-hard/
> story.
> So, it is not a rootkit or backup utility, it's a normal way Windows
> is working with files.
>
>>> The problem is that we don't have unix extensions for SMB 2.0 and
>>> higher protocol versions and we need to use nounix either way.
>>>
>>
>> Then that's a problem that will need to eventually be addressed anyway.
>> I don't see how this is anything but a way to paper over that fact in
>> the meantime.
>
> I really started to think that we can't use CreationTime in
> find_inode() since this is not constant on both Windows and Samba (by
> default). Even more, when negotiating Unix Extensions we always set
> this field to 0. I suggest that we need to do the same thing for
> nounix mounts too. At least we should do it for SMB 2/3 since we don't
> have Unix Extensions for them now.
>
> So, we have two choices from my point of view:
> 1) to update CreationTime value any time we are querying info from the
> server (what exactly this patch does);
> 2) to remove CreationTime checks in find_inode() for nounix at least
> for SMB 2/3 (probably for CIFS too).
>
> Thoughts?
>
> --
> Best regards,
> Pavel Shilovsky.
Would this problem be solved, if more than nounix, we restrict use/criterion
of cratetime in cifs_find_inode() for cases where server does not provide
uniqueids? Not sure how can we check that in cifs_find_inode() though.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
