hw/cxl: cdat: Fix failure to free buffer in erorr paths

The failure paths in CDAT file loading did not clear up properly.
Change to using g_auto_free and a local pointer for the buffer to
ensure this function has no side effects on error.
Also drop some unnecessary checks that can not fail.

Cleanup properly after a failure to load a CDAT file.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Message-Id: <20230421132020.7408-3-Jonathan.Cameron@huawei.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
Jonathan Cameron 2023-04-21 14:20:20 +01:00 committed by Michael S. Tsirkin
parent 71ba92f348
commit 7b22a3218a
3 changed files with 25 additions and 15 deletions

View file

@ -108,6 +108,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
static void ct3_load_cdat(CDATObject *cdat, Error **errp)
{
g_autofree CDATEntry *cdat_st = NULL;
g_autofree char *buf = NULL;
uint8_t sum = 0;
int num_ent;
int i = 0, ent = 1;
@ -116,7 +117,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp)
GError *error = NULL;
/* Read CDAT file and create its cache */
if (!g_file_get_contents(cdat->filename, (gchar **)&cdat->buf,
if (!g_file_get_contents(cdat->filename, (gchar **)&buf,
&file_size, &error)) {
error_setg(errp, "CDAT: File read failed: %s", error->message);
g_error_free(error);
@ -129,9 +130,17 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp)
i = sizeof(CDATTableHeader);
num_ent = 1;
while (i < file_size) {
hdr = (CDATSubHeader *)(cdat->buf + i);
hdr = (CDATSubHeader *)(buf + i);
if (i + sizeof(CDATSubHeader) > file_size) {
error_setg(errp, "CDAT: Truncated table");
return;
}
cdat_len_check(hdr, errp);
i += hdr->length;
if (i > file_size) {
error_setg(errp, "CDAT: Truncated table");
return;
}
num_ent++;
}
if (i != file_size) {
@ -139,33 +148,26 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp)
return;
}
cdat_st = g_malloc0(sizeof(*cdat_st) * num_ent);
if (!cdat_st) {
error_setg(errp, "CDAT: Failed to allocate entry array");
return;
}
cdat_st = g_new0(CDATEntry, num_ent);
/* Set CDAT header, Entry = 0 */
cdat_st[0].base = cdat->buf;
cdat_st[0].base = buf;
cdat_st[0].length = sizeof(CDATTableHeader);
i = 0;
while (i < cdat_st[0].length) {
sum += cdat->buf[i++];
sum += buf[i++];
}
/* Read CDAT structures */
while (i < file_size) {
hdr = (CDATSubHeader *)(cdat->buf + i);
cdat_len_check(hdr, errp);
hdr = (CDATSubHeader *)(buf + i);
cdat_st[ent].base = hdr;
cdat_st[ent].length = hdr->length;
while (cdat->buf + i <
(uint8_t *)cdat_st[ent].base + cdat_st[ent].length) {
while (buf + i < (char *)cdat_st[ent].base + cdat_st[ent].length) {
assert(i < file_size);
sum += cdat->buf[i++];
sum += buf[i++];
}
ent++;
@ -176,6 +178,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp)
}
cdat->entry_len = num_ent;
cdat->entry = g_steal_pointer(&cdat_st);
cdat->buf = g_steal_pointer(&buf);
}
void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp)

View file

@ -593,6 +593,9 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table;
cxl_cstate->cdat.private = ct3d;
cxl_doe_cdat_init(cxl_cstate, errp);
if (*errp) {
goto err_free_special_ops;
}
pcie_cap_deverr_init(pci_dev);
/* Leave a bit of room for expansion */
@ -605,6 +608,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
err_release_cdat:
cxl_doe_cdat_release(cxl_cstate);
err_free_special_ops:
g_free(regs->special_ops);
err_address_space_free:
address_space_destroy(&ct3d->hostmem_as);

View file

@ -346,6 +346,9 @@ static void cxl_usp_realize(PCIDevice *d, Error **errp)
cxl_cstate->cdat.free_cdat_table = free_default_cdat_table;
cxl_cstate->cdat.private = d;
cxl_doe_cdat_init(cxl_cstate, errp);
if (*errp) {
goto err_cap;
}
return;