Re: [PATCH v5 1/1] tools/hv: fcopy: Fix incorrect file path conversion

Naman Jain
2025-07-01T09:36:12Z

On 6/28/2025 7:52 AM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
>
> The hv_fcopy_uio_daemon fails to correctly handle file copy requests
> from Windows hosts (e.g. via Copy-VMFile) due to wchar_t size
> differences between Windows and Linux. On Linux, wchar_t is 32 bit,
> whereas Windows uses 16 bit wide characters.
>
> Fix this by ensuring that file transfers from host to Linux guest
> succeed with correctly decoded file names and paths.
>
> - Treats file name and path as __u16 arrays, not wchar_t*.
> - Allocates fixed-size buffers (W_MAX_PATH) for converted strings
> instead of using malloc.
> - Adds a check for target path length to prevent snprintf() buffer
> overflow.
>
> Fixes: 82b0945ce2c2 ("tools: hv: Add new fcopy application based on uio driver")
> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
> ---
> tools/hv/hv_fcopy_uio_daemon.c | 37 +++++++++++++---------------------
> 1 file changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d1..4b09ed6b6 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
>
> filesize = 0;
> p = path_name;
> - snprintf(target_fname, sizeof(target_fname), "%s/%s",
> - path_name, file_name);
> + if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
> + path_name, file_name) >= sizeof(target_fname)) {
> + syslog(LOG_ERR, "target file name is too long: %s/%s", path_name, file_name);
> + goto done;
> + }
>
> /*
> * Check to see if the path is already in place; if not,
> @@ -270,7 +273,7 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
> {
> size_t len = 0;
>
> - while (len < dest_size) {
> + while (len < dest_size && *src) {
> if (src[len] < 0x80)
> dest[len++] = (char)(*src++);
> else
> @@ -282,27 +285,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>
> static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
> {
> - setlocale(LC_ALL, "en_US.utf8");
> - size_t file_size, path_size;
> - char *file_name, *path_name;
> - char *in_file_name = (char *)smsg_in->file_name;
> - char *in_path_name = (char *)smsg_in->path_name;
> -
> - file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
> - path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
> -
> - file_name = (char *)malloc(file_size * sizeof(char));
> - path_name = (char *)malloc(path_size * sizeof(char));
> -
> - if (!file_name || !path_name) {
> - free(file_name);
> - free(path_name);
> - syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
> - return HV_E_FAIL;
> - }
> + /*
> + * file_name and path_name should have same length with appropriate
> + * member of hv_start_fcopy.
> + */
> + char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>
> - wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
> - wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
> + setlocale(LC_ALL, "en_US.utf8");
> + wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
> + wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>
> return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
> }

LGTM. FYI, Fcopy daemon is broken on some systems currently. Below
change should fix it:

https://lore.kernel.org/all/20250620070618.3097-1-namjain@linux.microsoft.com/

Reviewed-by: Naman Jain <namjain@linux.microsoft.com>