Use hard links to link sysroot files within ModuleCache.

[ovyalov]

Summary

Use hard links to link sysroot files within ModuleCache because sym links on Windows require SE_CREATE_SYMBOLIC_LINK_NAME privilege enabled.

[zturner]

Even with this check, creating the symbolic link might still fail for other reasons related to the filesystem and not related to security. Would it be better to just try to create it no matter what and log the error if it fails?

[ovyalov]

In D9587#168485, @zturner wrote:

Even with this check, creating the symbolic link might still fail for other reasons related to the filesystem and not related to security. Would it be better to just try to create it no matter what and log the error if it fails?

I believe calling EnableCreateSymLinkPrivilege might be beneficial anyway since as I can tell from running "whoami /priv" that SE_CREATE_SYMBOLIC_LINK_NAME is disabled by default . I'm somewhat concerned that we may call FileSystem::Symlink when we 100% confident that it will fail. As an alternative, we may just call FileSystem::Symlink anyway but always ignoring its result...

Please let me know your opinion.

[zturner]

Hmm, yea that's not a bad idea. Let me look into the privilege thing a
little tomorrow. In the meantime, do we properly handle the error case
anyway (at least it should print a log and not have unexpected behavior).
Creating the symlink could fail on other platforms too, not just windows

[ovyalov]

Consolidated all logic that pertains to sysroot management in CreateHostSysRootModuleSymLink.

[clayborg]

Is there really no other way to create something akin to a symlink on windows? How can you have control over a directory and yet not be able to create links? Is there any other kind if link? Alias? anything? Seems weird to be able to create files but not create links in a directory you have created?

[chaoren]

Would a .lnk file work? I think the Windows API should contain functions to
create/resolve those.

[zturner]

In D9587#169185, @clayborg wrote:

Is there really no other way to create something akin to a symlink on windows? How can you have control over a directory and yet not be able to create links? Is there any other kind if link? Alias? anything? Seems weird to be able to create files but not create links in a directory you have created?

It really depends on what you mean by control. NTFS's security complicated is far more granular than that of other file systems. That said, Windows has many different types of links. Only symlinks have this restriction, and I don't know why it is the way it is. It looks like this whose target is a file. If that is the case, you might consider using a hardlink instead. There are no special privileges required to create a hardlink on Windows. You could even have it use a hardlink on windows and a symlink on other platforms, if that seems better.
I think this is the easiest and most-likely-to-work-in-the-most-scenarios approach as long as it works for what you need. What do you think Oleksiy?

[zturner]

In D9587#169194, @chaoren wrote:

Would a .lnk file work? I think the Windows API should contain functions to
create/resolve those.

.lnk files are only understood by the shell, not by the file system. So you can execute a link file by calling ShellExecute (a shell function), but you can't CreateProcess (a system call) on one. You can manually resolve one, but as youc an see from looking at this code, it's not exactly a fun thing to do: https://msdn.microsoft.com/en-us/library/windows/desktop/bb776891%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396#Shellink_Resolving_Shortcut

[ovyalov]

In D9587#169224, @zturner wrote:

In D9587#169185, @clayborg wrote:

Is there really no other way to create something akin to a symlink on windows? How can you have control over a directory and yet not be able to create links? Is there any other kind if link? Alias? anything? Seems weird to be able to create files but not create links in a directory you have created?

It really depends on what you mean by control. NTFS's security complicated is far more granular than that of other file systems. That said, Windows has many different types of links. Only symlinks have this restriction, and I don't know why it is the way it is. It looks like this whose target is a file. If that is the case, you might consider using a hardlink instead. There are no special privileges required to create a hardlink on Windows. You could even have it use a hardlink on windows and a symlink on other platforms, if that seems better.
I think this is the easiest and most-likely-to-work-in-the-most-scenarios approach as long as it works for what you need. What do you think Oleksiy?

If hard links don't need any security requirements it should be ok. I can create new call FileSystem::Hardlink (with use of link call in Posix and CreateHardLink on Windows) and switch my code to use it instead of SymLink.

[ovyalov]

Added FileSystem::HardLink call and switched ModuleCache to use it.

[zturner]

Looks good. Glad we avoided getting all that token stuff into LLDB. I suspect we may need to do that in the future for other reasons, but it's scary stuff so I'm glad we can hold out a little longer.

[clayborg]

Looks good.

[ovyalov]

AFFECTED FILES

/lldb/trunk/include/lldb/Host/FileSystem.h
/lldb/trunk/source/Host/posix/FileSystem.cpp
/lldb/trunk/source/Host/windows/FileSystem.cpp
/lldb/trunk/source/Utility/ModuleCache.cpp
/lldb/trunk/source/Utility/ModuleCache.h

USERS

ovyalov (Author)

http://reviews.llvm.org/rL236917

include/lldb/Host/FileSystem.h

public:
     static Error MakeDirectory(const char *path, uint32_t mode);
     static Error DeleteDirectory(const char *path, bool recurse);
 
     static Error GetFilePermissions(const char *path, uint32_t &file_permissions);
     static Error SetFilePermissions(const char *path, uint32_t file_permissions);
     static lldb::user_id_t GetFileSize(const FileSpec &file_spec);
     static bool GetFileExists(const FileSpec &file_spec);
 
+    static Error Hardlink(const char *src, const char *dst);
     static Error Symlink(const char *src, const char *dst);
     static Error Readlink(const char *path, char *buf, size_t buf_len);
     static Error Unlink(const char *path);
 
     static bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high);
     static bool CalculateMD5(const FileSpec &file_spec,
                              uint64_t offset,
                              uint64_t length,

source/Host/posix/FileSystem.cpp

 
 bool
 FileSystem::GetFileExists(const FileSpec &file_spec)
 {
     return file_spec.Exists();
 }
 
 Error
+FileSystem::Hardlink(const char *src, const char *dst)
+{
+    Error error;
+    if (::link(dst, src) == -1)
+        error.SetErrorToErrno();
+    return error;
+}
+
+Error
 FileSystem::Symlink(const char *src, const char *dst)
 {
     Error error;
     if (::symlink(dst, src) == -1)
         error.SetErrorToErrno();
     return error;
 }
 

source/Host/windows/FileSystem.cpp

 
 bool
 FileSystem::GetFileExists(const FileSpec &file_spec)
 {
     return file_spec.Exists();
 }
 
 Error
+FileSystem::Hardlink(const char *linkname, const char *target)
+{
+    Error error;
+    if (!::CreateHardLink(linkname, target, nullptr))
+        error.SetError(::GetLastError(), lldb::eErrorTypeWin32);
+    return error;
+}
+
+Error
 FileSystem::Symlink(const char *linkname, const char *target)
 {
     Error error;
     DWORD attrib = ::GetFileAttributes(target);
     if (attrib == INVALID_FILE_ATTRIBUTES)
     {
         error.SetError(::GetLastError(), lldb::eErrorTypeWin32);
         return error;

source/Utility/ModuleCache.h

private:
 
     Error
     Get (const FileSpec &root_dir_spec,
          const char *hostname,
          const ModuleSpec &module_spec,
          lldb::ModuleSP &cached_module_sp,
          bool *did_create_ptr);
 
-    static FileSpec
-    GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid);
-
-    static FileSpec
-    GetHostSysRootModulePath (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec);
-
-    static Error
-    CreateHostSysRootModuleSymLink (const FileSpec &sysroot_module_path_spec, const FileSpec &module_file_path);
-
     std::unordered_map<std::string, lldb::ModuleWP> m_loaded_modules;
 };
 
 } // namespace lldb_private
 
 #endif  // utility_ModuleCache_h_

source/Utility/ModuleCache.cpp

if (dir_path.Exists ())
 
         return Error ();
     }
 
     return FileSystem::MakeDirectory (dir_path.GetPath ().c_str (),
                                       eFilePermissionsDirectoryDefault);
 }
 
+FileSpec
+GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid)
+{
+    const auto modules_dir_spec = JoinPath (root_dir_spec, kModulesSubdir);
+    return JoinPath (modules_dir_spec, uuid.GetAsString ().c_str ());
+}
+
+Error
+CreateHostSysRootModuleLink (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec, const FileSpec &local_module_spec)
+{
+    const auto sysroot_module_path_spec = JoinPath (
+        JoinPath (root_dir_spec, hostname), platform_module_spec.GetPath ().c_str ());
+    if (sysroot_module_path_spec.Exists())
+        return Error ();
+
+    const auto error = MakeDirectory (FileSpec (sysroot_module_path_spec.GetDirectory ().AsCString (), false));
+    if (error.Fail ())
+        return error;
+
+    return FileSystem::Hardlink (sysroot_module_path_spec.GetPath ().c_str (), local_module_spec.GetPath ().c_str ());
+}
+
 }  // namespace
 
 Error
 ModuleCache::Put (const FileSpec &root_dir_spec,
                   const char *hostname,
                   const ModuleSpec &module_spec,
                   const FileSpec &tmp_file)
 {
     const auto module_spec_dir = GetModuleDirectory (root_dir_spec, module_spec.GetUUID ());
     const auto module_file_path = JoinPath (module_spec_dir, module_spec.GetFileSpec ().GetFilename ().AsCString ());
 
     const auto tmp_file_path = tmp_file.GetPath ();
     const auto err_code = llvm::sys::fs::rename (tmp_file_path.c_str (), module_file_path.GetPath ().c_str ());
     if (err_code)
         return Error ("Failed to rename file %s to %s: %s",
                       tmp_file_path.c_str (), module_file_path.GetPath ().c_str (), err_code.message ().c_str ());
 
-    // Create sysroot link to a module.
-    const auto sysroot_module_path_spec = GetHostSysRootModulePath (root_dir_spec, hostname, module_spec.GetFileSpec ());
-    return CreateHostSysRootModuleSymLink (sysroot_module_path_spec, module_file_path);
+    const auto error = CreateHostSysRootModuleLink(root_dir_spec, hostname, module_spec.GetFileSpec(), module_file_path);
+    if (error.Fail ())
+        return Error ("Failed to create link to %s: %s", module_file_path.GetPath ().c_str (), error.AsCString ());
+    return Error ();
 }
 
 Error
 ModuleCache::Get (const FileSpec &root_dir_spec,
                   const char *hostname,
                   const ModuleSpec &module_spec,
                   ModuleSP &cached_module_sp,
                   bool *did_create_ptr)
 {
     const auto find_it = m_loaded_modules.find (module_spec.GetUUID ().GetAsString());
     if (find_it != m_loaded_modules.end ())
     {
         cached_module_sp = (*find_it).second.lock ();
         if (cached_module_sp)
             return Error ();
         m_loaded_modules.erase (find_it);
     }
 
-    const auto module_spec_dir = GetModuleDirectory (root_dir_spec,  module_spec.GetUUID ());
+    const auto module_spec_dir = GetModuleDirectory (root_dir_spec, module_spec.GetUUID ());
     const auto module_file_path = JoinPath (module_spec_dir, module_spec.GetFileSpec ().GetFilename ().AsCString ());
 
     if (!module_file_path.Exists ())
         return Error ("Module %s not found", module_file_path.GetPath ().c_str ());
     if (module_file_path.GetByteSize () != module_spec.GetObjectSize ())
         return Error ("Module %s has invalid file size", module_file_path.GetPath ().c_str ());
 
-    // We may have already cached module but downloaded from an another host - in this case let's create a symlink to it.
-    const auto sysroot_module_path_spec = GetHostSysRootModulePath (root_dir_spec, hostname, module_spec.GetFileSpec ());
-    if (!sysroot_module_path_spec.Exists ())
-        CreateHostSysRootModuleSymLink (sysroot_module_path_spec, module_spec.GetFileSpec ());
+    // We may have already cached module but downloaded from an another host - in this case let's create a link to it.
+    const auto error = CreateHostSysRootModuleLink(root_dir_spec, hostname, module_spec.GetFileSpec(), module_file_path);
+    if (error.Fail ())
+        return Error ("Failed to create link to %s: %s", module_file_path.GetPath().c_str(), error.AsCString());
 
     auto cached_module_spec (module_spec);
     cached_module_spec.GetUUID ().Clear ();  // Clear UUID since it may contain md5 content hash instead of real UUID.
     cached_module_spec.GetFileSpec () = module_file_path;
     cached_module_spec.GetPlatformFileSpec () = module_spec.GetFileSpec ();
     cached_module_sp.reset (new Module (cached_module_spec));
     if (did_create_ptr)
         *did_create_ptr = true;
ModuleCache::GetAndPut (const FileSpec &root_dir_spec,
     // Put downloaded file into local module cache.
     error = Put (root_dir_spec, hostname, module_spec, tmp_download_file_spec);
     if (error.Fail ())
         return Error ("Failed to put module into cache: %s", error.AsCString ());
 
     tmp_file_remover.releaseFile ();
     return Get (root_dir_spec, hostname, module_spec, cached_module_sp, did_create_ptr);
 }
-
-FileSpec
-ModuleCache::GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid)
-{
-    const auto modules_dir_spec = JoinPath (root_dir_spec, kModulesSubdir);
-    return JoinPath (modules_dir_spec, uuid.GetAsString ().c_str ());
-}
-
-FileSpec
-ModuleCache::GetHostSysRootModulePath (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec)
-{
-    const auto sysroot_dir = JoinPath (root_dir_spec, hostname);
-    return JoinPath (sysroot_dir, platform_module_spec.GetPath ().c_str ());
-}
-
-Error
-ModuleCache::CreateHostSysRootModuleSymLink (const FileSpec &sysroot_module_path_spec, const FileSpec &module_file_path)
-{
-    const auto error = MakeDirectory (FileSpec (sysroot_module_path_spec.GetDirectory ().AsCString (), false));
-    if (error.Fail ())
-        return error;
-
-    return FileSystem::Symlink (sysroot_module_path_spec.GetPath ().c_str (),
-                                module_file_path.GetPath ().c_str ());
-}