Skip to content

Commit

Permalink
Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20220…
Browse files Browse the repository at this point in the history
…210' into staging

9pfs: fixes and cleanup

* Fifth patch fixes a 9pfs server crash that happened on some systems due
  to incorrect (system dependant) handling of struct dirent size.

* Tests: Second patch fixes a test error that happened on some systems due
  mkdir() being called twice for creating the test directory for the 9p
  'local' tests.

* Tests: Third patch fixes a memory leak.

* Tests: The remaining two patches are code cleanup.

# gpg: Signature made Thu 10 Feb 2022 11:21:38 GMT
# gpg:                using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395
# gpg:                issuer "qemu_oss@crudebyte.com"
# gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: ECAB 1A45 4014 1413 BA38  4926 30DB 47C3 A012 D5F4
#      Subkey fingerprint: 96D8 D110 CF7A F808 4F88  5901 34C2 B587 65A4 7395

* remotes/cschoenebeck/tags/pull-9p-20220210:
  9pfs: Fix segfault in do_readdir_many caused by struct dirent overread
  tests/9pfs: Use g_autofree and g_autoptr where possible
  tests/9pfs: Fix leak of local_test_path
  tests/9pfs: fix mkdir() being called twice
  tests/9pfs: use g_autofree where possible

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
  • Loading branch information
pm215 committed Feb 13, 2022
2 parents 48033ad + de19c79 commit c011e4a
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 89 deletions.
3 changes: 1 addition & 2 deletions hw/9pfs/codir.c
Expand Up @@ -143,8 +143,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp,
} else {
e = e->next = g_malloc0(sizeof(V9fsDirEnt));
}
e->dent = g_malloc0(sizeof(struct dirent));
memcpy(e->dent, dent, sizeof(struct dirent));
e->dent = qemu_dirent_dup(dent);

/* perform a full stat() for directory entry if requested by caller */
if (dostat) {
Expand Down
13 changes: 13 additions & 0 deletions include/qemu/osdep.h
Expand Up @@ -805,6 +805,19 @@ static inline int platform_does_not_support_system(const char *command)
}
#endif /* !HAVE_SYSTEM_FUNCTION */

/**
* Duplicate directory entry @dent.
*
* It is highly recommended to use this function instead of open coding
* duplication of @c dirent objects, because the actual @c struct @c dirent
* size may be bigger or shorter than @c sizeof(struct dirent) and correct
* handling is platform specific (see gitlab issue #841).
*
* @dent - original directory entry to be duplicated
* @returns duplicated directory entry which should be freed with g_free()
*/
struct dirent *qemu_dirent_dup(struct dirent *dent);

#ifdef __cplusplus
}
#endif
Expand Down
38 changes: 14 additions & 24 deletions tests/qtest/libqos/virtio-9p.c
Expand Up @@ -37,31 +37,23 @@ static char *concat_path(const char* a, const char* b)
return g_build_filename(a, b, NULL);
}

static void init_local_test_path(void)
void virtio_9p_create_local_test_dir(void)
{
char *pwd = g_get_current_dir();
g_assert(local_test_path == NULL);
struct stat st;
g_autofree char *pwd = g_get_current_dir();
/*
* template gets cached into local_test_path and freed in
* virtio_9p_remove_local_test_dir().
*/
char *template = concat_path(pwd, "qtest-9p-local-XXXXXX");

local_test_path = mkdtemp(template);
if (!local_test_path) {
g_test_message("mkdtemp('%s') failed: %s", template, strerror(errno));
}
g_assert(local_test_path);
g_free(pwd);
}

void virtio_9p_create_local_test_dir(void)
{
struct stat st;
int res;

init_local_test_path();

g_assert(local_test_path != NULL);
res = mkdir(local_test_path, 0777);
if (res < 0) {
g_test_message("mkdir('%s') failed: %s", local_test_path,
strerror(errno));
}

/* ensure test directory exists now ... */
g_assert(stat(local_test_path, &st) == 0);
Expand All @@ -72,12 +64,13 @@ void virtio_9p_create_local_test_dir(void)
void virtio_9p_remove_local_test_dir(void)
{
g_assert(local_test_path != NULL);
char *cmd = g_strdup_printf("rm -fr '%s'\n", local_test_path);
g_autofree char *cmd = g_strdup_printf("rm -fr '%s'\n", local_test_path);
int res = system(cmd);
if (res < 0) {
/* ignore error, dummy check to prevent compiler error */
}
g_free(cmd);
g_free(local_test_path);
local_test_path = NULL;
}

char *virtio_9p_test_path(const char *path)
Expand Down Expand Up @@ -221,8 +214,8 @@ static void *virtio_9p_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
static void regex_replace(GString *haystack, const char *pattern,
const char *replace_fmt, ...)
{
GRegex *regex;
char *replace, *s;
g_autoptr(GRegex) regex = NULL;
g_autofree char *replace = NULL, *s = NULL;
va_list argp;

va_start(argp, replace_fmt);
Expand All @@ -232,9 +225,6 @@ static void regex_replace(GString *haystack, const char *pattern,
regex = g_regex_new(pattern, 0, 0, NULL);
s = g_regex_replace(regex, haystack->str, -1, 0, replace, 0, NULL);
g_string_assign(haystack, s);
g_free(s);
g_regex_unref(regex);
g_free(replace);
}

void virtio_9p_assign_local_driver(GString *cmd_line, const char *args)
Expand Down

0 comments on commit c011e4a

Please sign in to comment.