Duplicate SMB file_ids leading to Windows client cache poisoning

[Jeremy Allison]

On Tue, Dec 14, 2021 at 05:40:45PM -0500, J. Bruce Fields wrote:
>
>Even if you could: it's true that there's an i_generation in the generic
>struct inode, but I doubt there's any guarantee it meets your
>requirements (or is even initialized to anything at all) for a given
>filesystem.
>
>(Whereas if it were added to statx then the filesystem could choose to
>return it or not depending on whether it's supported.)

So to step back a bit, I think this only affects MacOSX clients.

I believe (Tom please correct me if I'm wrong) the Windows buffer
cache is based on pathname from the share root, not the reported fileid.

Also, I'm guessing that the buffer cache should not be preserved
over a connection loss. Or is it preserved if the client has a
persistent handle ?

So from the server side we have a 64-bit number we need to keep
constant over the lifetime of a particular file, and at least
for MacOSX clients we can't re-use any number even if the file
is deleted and re-created over the lifetime of a connection.

I'm just trying to scope out the exact issue we're trying
to solve.

Currently in Samba, we have a 64-bit fileid, which
we return via the following call in the VFS.

uint64_t fileid = SMB_VFS_FS_FILE_ID(conn, &stat_struct);

The default is:

vfswrap_fs_file_id(), which looks like:

static uint64_t vfswrap_fs_file_id(struct vfs_handle_struct *handle,
                                    const SMB_STRUCT_STAT *psbuf)
{
        uint64_t file_id;

         if (!(psbuf->st_ex_iflags & ST_EX_IFLAG_CALCULATED_FILE_ID)) {
                 return psbuf->st_ex_file_id;
         }

         if (handle->conn->base_share_dev == psbuf->st_ex_dev) {
                 return (uint64_t)psbuf->st_ex_ino;
         }

         /* FileIDLow */
         file_id = ((psbuf->st_ex_ino) & UINT32_MAX);

         /* FileIDHigh */
         file_id |= ((uint64_t)((psbuf->st_ex_dev) & UINT32_MAX)) << 32;

         return file_id;
}

By default, when reading the stat info from a file,
st_ex_file_id is set directly from the inode number.

         st_ex_file_id = ex_ino;
         st_ex_iflags |= ST_EX_IFLAG_CALCULATED_FILE_ID;

and the flag ST_EX_IFLAG_CALCULATED_FILE_ID is set to
show this is a 'calculated from inode' fileid.

When smbd itself creates a file or directory, it calls:

                         file_id = make_file_id_from_itime(&smb_dname->st);
                         update_stat_ex_file_id(&smb_dname->st, file_id);

where make_file_id_from_itime() returns the immutable "birth time"
as the fileid, and update_stat_ex_file_id() does:

         st_ex_file_id = file_id;
         st_ex_iflags &= ~ST_EX_IFLAG_CALCULATED_FILE_ID;

so it clears the ST_EX_IFLAG_CALCULATED_FILE_ID flag,
meaning that vfswrap_fs_file_id() will directly return
st_ex_file_id (i.e. the immutable "birth time") as
the fileid.

These two calls (make_file_id_from_itime()/update_stat_ex_file_id())
are also done when smbd reads an extended attribute containing
a stored DOS attribute with the XATTR_DOSINFO_ITIME flag set,
meaning we always return the same fileid for a file with
a given birthtime.

This fixes the problem for MacOSX clients (in that
birthtime is always increasing as the arrow of time
marches on) but then fails for the case where the
birthtime timestamp resolution is so poor that two
files can end up with the same birthtime.

I'm thinking a tweak in the make_file_id_from_itime()
case must be the eventual fix here.

Still considering the right way to go about this...