* [PATCH v5 0/1] tools/hv: fcopy: Fix incorrect file path conversion @ 2025-06-28 2:22 yasuenag 2025-06-28 2:22 ` [PATCH v5 1/1] " yasuenag 0 siblings, 1 reply; 4+ messages in thread From: yasuenag @ 2025-06-28 2:22 UTC (permalink / raw) To: namjain Cc: eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar, Yasumasa Suenaga From: Yasumasa Suenaga <yasuenag@gmail.com> Hi Naman, Thanks a lot for your advise! I updated commit message. This version does not have any code changes from v4. I hope this patch would go well. Best regards, Yasumasa Yasumasa Suenaga (1): tools/hv: fcopy: Fix incorrect file path conversion tools/hv/hv_fcopy_uio_daemon.c | 37 +++++++++++++--------------------- 1 file changed, 14 insertions(+), 23 deletions(-) -- 2.49.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v5 1/1] tools/hv: fcopy: Fix incorrect file path conversion 2025-06-28 2:22 [PATCH v5 0/1] tools/hv: fcopy: Fix incorrect file path conversion yasuenag @ 2025-06-28 2:22 ` yasuenag 2025-07-01 9:36 ` Naman Jain 0 siblings, 1 reply; 4+ messages in thread From: yasuenag @ 2025-06-28 2:22 UTC (permalink / raw) To: namjain Cc: eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar, Yasumasa Suenaga 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); } -- 2.49.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v5 1/1] tools/hv: fcopy: Fix incorrect file path conversion 2025-06-28 2:22 ` [PATCH v5 1/1] " yasuenag @ 2025-07-01 9:36 ` Naman Jain 2025-07-09 23:02 ` Wei Liu 0 siblings, 1 reply; 4+ messages in thread From: Naman Jain @ 2025-07-01 9:36 UTC (permalink / raw) To: yasuenag; +Cc: eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5 1/1] tools/hv: fcopy: Fix incorrect file path conversion 2025-07-01 9:36 ` Naman Jain @ 2025-07-09 23:02 ` Wei Liu 0 siblings, 0 replies; 4+ messages in thread From: Wei Liu @ 2025-07-09 23:02 UTC (permalink / raw) To: Naman Jain Cc: yasuenag, eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar On Tue, Jul 01, 2025 at 03:06:08PM +0530, Naman Jain wrote: > > > 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> [...] > Reviewed-by: Naman Jain <namjain@linux.microsoft.com> Applied to hyperv-fixes. Thank you both for the patch and the review! ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-07-09 23:02 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-28 2:22 [PATCH v5 0/1] tools/hv: fcopy: Fix incorrect file path conversion yasuenag 2025-06-28 2:22 ` [PATCH v5 1/1] " yasuenag 2025-07-01 9:36 ` Naman Jain 2025-07-09 23:02 ` Wei Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox