The read() syscall is not guaranteed to return all data from a file. The
default ROM loader implementation currently does not take this into account,
instead failing if all bytes are not read at once. This change loads the ROM
using g_file_get_contents() instead, which correctly reads all data using
multiple calls to read() while also returning the loaded ROM size.
Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com>
---
hw/core/loader.c | 30 +++++-------------------------
1 file changed, 5 insertions(+), 25 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2f8105d7de..44444a5714 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1075,8 +1075,7 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
{
MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
Rom *rom;
- ssize_t rc;
- int fd = -1;
+ g_autoptr(GError) gerr = NULL;
char devpath[100];
if (as && mr) {
@@ -1094,35 +1093,19 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
rom->path = g_strdup(file);
}
- fd = open(rom->path, O_RDONLY | O_BINARY);
- if (fd == -1) {
- fprintf(stderr, "Could not open option rom '%s': %s\n",
- rom->path, strerror(errno));
- goto err;
- }
-
if (fw_dir) {
rom->fw_dir = g_strdup(fw_dir);
rom->fw_file = g_strdup(file);
}
rom->addr = addr;
- rom->romsize = lseek(fd, 0, SEEK_END);
- if (rom->romsize == -1) {
- fprintf(stderr, "rom: file %-20s: get size error: %s\n",
- rom->name, strerror(errno));
+ if (!g_file_get_contents(rom->path, (gchar **) &rom->data,
+ &rom->romsize, &gerr)) {
+ fprintf(stderr, "rom: file %-20s: error %s\n",
+ rom->name, gerr->message);
goto err;
}
rom->datasize = rom->romsize;
- rom->data = g_malloc0(rom->datasize);
- lseek(fd, 0, SEEK_SET);
- rc = read(fd, rom->data, rom->datasize);
- if (rc != rom->datasize) {
- fprintf(stderr, "rom: file %-20s: read error: rc=%zd (expected %zd)\n",
- rom->name, rc, rom->datasize);
- goto err;
- }
- close(fd);
rom_insert(rom);
if (rom->fw_file && fw_cfg) {
const char *basename;
@@ -1159,9 +1142,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
return 0;
err:
- if (fd != -1)
- close(fd);
-
rom_free(rom);
return -1;
}
--
2.45.2
On 28/6/24 20:27, Gregor Haas wrote: > The read() syscall is not guaranteed to return all data from a file. The > default ROM loader implementation currently does not take this into account, > instead failing if all bytes are not read at once. This change loads the ROM > using g_file_get_contents() instead, which correctly reads all data using > multiple calls to read() while also returning the loaded ROM size. > > Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com> > --- > hw/core/loader.c | 30 +++++------------------------- > 1 file changed, 5 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> and queued, thanks!
Reviewed-by: Xingtao Yao <yaoxt.fnst@fujitsu.com> > -----Original Message----- > From: Gregor Haas <gregorhaas1997@gmail.com> > Sent: Saturday, June 29, 2024 2:27 AM > To: qemu-devel@nongnu.org > Cc: berrange@redhat.com; Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; > Gregor Haas <gregorhaas1997@gmail.com> > Subject: [PATCH v4] hw/core/loader: allow loading larger ROMs > > The read() syscall is not guaranteed to return all data from a file. The > default ROM loader implementation currently does not take this into account, > instead failing if all bytes are not read at once. This change loads the ROM > using g_file_get_contents() instead, which correctly reads all data using > multiple calls to read() while also returning the loaded ROM size. > > Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com> > --- > hw/core/loader.c | 30 +++++------------------------- > 1 file changed, 5 insertions(+), 25 deletions(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 2f8105d7de..44444a5714 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -1075,8 +1075,7 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, > { > MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); > Rom *rom; > - ssize_t rc; > - int fd = -1; > + g_autoptr(GError) gerr = NULL; > char devpath[100]; > > if (as && mr) { > @@ -1094,35 +1093,19 @@ ssize_t rom_add_file(const char *file, const char > *fw_dir, > rom->path = g_strdup(file); > } > > - fd = open(rom->path, O_RDONLY | O_BINARY); > - if (fd == -1) { > - fprintf(stderr, "Could not open option rom '%s': %s\n", > - rom->path, strerror(errno)); > - goto err; > - } > - > if (fw_dir) { > rom->fw_dir = g_strdup(fw_dir); > rom->fw_file = g_strdup(file); > } > rom->addr = addr; > - rom->romsize = lseek(fd, 0, SEEK_END); > - if (rom->romsize == -1) { > - fprintf(stderr, "rom: file %-20s: get size error: %s\n", > - rom->name, strerror(errno)); > + if (!g_file_get_contents(rom->path, (gchar **) &rom->data, > + &rom->romsize, &gerr)) { > + fprintf(stderr, "rom: file %-20s: error %s\n", > + rom->name, gerr->message); > goto err; > } > > rom->datasize = rom->romsize; > - rom->data = g_malloc0(rom->datasize); > - lseek(fd, 0, SEEK_SET); > - rc = read(fd, rom->data, rom->datasize); > - if (rc != rom->datasize) { > - fprintf(stderr, "rom: file %-20s: read error: rc=%zd (expected %zd)\n", > - rom->name, rc, rom->datasize); > - goto err; > - } > - close(fd); > rom_insert(rom); > if (rom->fw_file && fw_cfg) { > const char *basename; > @@ -1159,9 +1142,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, > return 0; > > err: > - if (fd != -1) > - close(fd); > - > rom_free(rom); > return -1; > } > -- > 2.45.2
On Fri, Jun 28, 2024 at 11:27:06AM -0700, Gregor Haas wrote: > The read() syscall is not guaranteed to return all data from a file. The > default ROM loader implementation currently does not take this into account, > instead failing if all bytes are not read at once. This change loads the ROM > using g_file_get_contents() instead, which correctly reads all data using > multiple calls to read() while also returning the loaded ROM size. > > Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com> > --- > hw/core/loader.c | 30 +++++------------------------- > 1 file changed, 5 insertions(+), 25 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.