Message ID | 20210903174510.751630-8-philmd@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v3,01/28] hw/hyperv/vmbus: Remove unused vmbus_load/save_req() | expand |
On Freitag, 3. September 2021 19:44:49 CEST Philippe Mathieu-Daudé wrote: > Per > https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-n > ow/5538 > > The old API took the size of the memory to duplicate as a guint, > whereas most memory functions take memory sizes as a gsize. This > made it easy to accidentally pass a gsize to g_memdup(). For large > values, that would lead to a silent truncation of the size from 64 > to 32 bits, and result in a heap area being returned which is > significantly smaller than what the caller expects. This can likely > be exploited in various modules to cause a heap buffer overflow. > > Replace g_memdup() by the safer g_memdup2() wrapper. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/9pfs/9p-synth.c | 2 +- > hw/9pfs/9p.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c > index b38088e0664..d6168c653d2 100644 > --- a/hw/9pfs/9p-synth.c > +++ b/hw/9pfs/9p-synth.c > @@ -497,7 +497,7 @@ static int synth_name_to_path(FsContext *ctx, V9fsPath > *dir_path, out: > /* Copy the node pointer to fid */ > g_free(target->data); > - target->data = g_memdup(&node, sizeof(void *)); > + target->data = g_memdup2(&node, sizeof(void *)); > target->size = sizeof(void *); > return 0; > } That's Ok, trivial change. > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index c857b313213..a80166fcaff 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -202,7 +202,7 @@ void v9fs_path_copy(V9fsPath *dst, const V9fsPath *src) > { > v9fs_path_free(dst); > dst->size = src->size; > - dst->data = g_memdup(src->data, src->size); > + dst->data = g_memdup2(src->data, src->size); > } > > int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath, src->size is actually just 16 bit (fsdev/file-op-9p.h): struct V9fsPath { uint16_t size; char *data; }; Should (still) be Ok as well as V9fsPath is about file system pathes which are currently limited to 4k (PATH_MAX). Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Best regards, Christian Schoenebeck
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c index b38088e0664..d6168c653d2 100644 --- a/hw/9pfs/9p-synth.c +++ b/hw/9pfs/9p-synth.c @@ -497,7 +497,7 @@ static int synth_name_to_path(FsContext *ctx, V9fsPath *dir_path, out: /* Copy the node pointer to fid */ g_free(target->data); - target->data = g_memdup(&node, sizeof(void *)); + target->data = g_memdup2(&node, sizeof(void *)); target->size = sizeof(void *); return 0; } diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index c857b313213..a80166fcaff 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -202,7 +202,7 @@ void v9fs_path_copy(V9fsPath *dst, const V9fsPath *src) { v9fs_path_free(dst); dst->size = src->size; - dst->data = g_memdup(src->data, src->size); + dst->data = g_memdup2(src->data, src->size); } int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
Per https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538 The old API took the size of the memory to duplicate as a guint, whereas most memory functions take memory sizes as a gsize. This made it easy to accidentally pass a gsize to g_memdup(). For large values, that would lead to a silent truncation of the size from 64 to 32 bits, and result in a heap area being returned which is significantly smaller than what the caller expects. This can likely be exploited in various modules to cause a heap buffer overflow. Replace g_memdup() by the safer g_memdup2() wrapper. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/9pfs/9p-synth.c | 2 +- hw/9pfs/9p.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)