[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [for-2.11 PATCH 03/26] spapr_iommu: use g_st
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [for-2.11 PATCH 03/26] spapr_iommu: use g_strdup_printf() instead of snprintf() |
Date: |
Wed, 26 Jul 2017 11:48:46 +0200 |
On Wed, 26 Jul 2017 13:37:03 +1000
Alexey Kardashevskiy <address@hidden> wrote:
> On 26/07/17 03:58, Greg Kurz wrote:
> > Passing a stack allocated buffer of arbitrary length to snprintf()
> > without checking the return value can cause the resultant strings
> > to be silently truncated.
>
> The strings it is touching cannot be silently truncated as
> "tce-iommu-XXXXXXXX" are shorter than 32 chars.
>
True but if the string was to be changed (unlikely, I admit) then we should
ensure the array is large enough. And anyway, this means we waste stack space,
which is suboptimal. As noted by David, it is a common practice in QEMU to use
g_strdup_printf().
>
> >
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > hw/ppc/spapr_iommu.c | 13 ++++++++-----
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> > index e614621a8317..740d42608b61 100644
> > --- a/hw/ppc/spapr_iommu.c
> > +++ b/hw/ppc/spapr_iommu.c
> > @@ -252,17 +252,19 @@ static int spapr_tce_table_realize(DeviceState *dev)
> > {
> > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
> > Object *tcetobj = OBJECT(tcet);
> > - char tmp[32];
> > + gchar *tmp;
> >
> > tcet->fd = -1;
> > tcet->need_vfio = false;
> > - snprintf(tmp, sizeof(tmp), "tce-root-%x", tcet->liobn);
> > + tmp = g_strdup_printf("tce-root-%x", tcet->liobn);
> > memory_region_init(&tcet->root, tcetobj, tmp, UINT64_MAX);
> > + g_free(tmp);
> >
> > - snprintf(tmp, sizeof(tmp), "tce-iommu-%x", tcet->liobn);
> > + tmp = g_strdup_printf("tce-iommu-%x", tcet->liobn);
> > memory_region_init_iommu(&tcet->iommu, sizeof(tcet->iommu),
> > TYPE_SPAPR_IOMMU_MEMORY_REGION,
> > tcetobj, tmp, 0);
> > + g_free(tmp);
> >
> > QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
> >
> > @@ -307,7 +309,7 @@ void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool
> > need_vfio)
> > sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
> > {
> > sPAPRTCETable *tcet;
> > - char tmp[32];
> > + gchar *tmp;
> >
> > if (spapr_tce_find_by_liobn(liobn)) {
> > error_report("Attempted to create TCE table with duplicate"
> > @@ -318,8 +320,9 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner,
> > uint32_t liobn)
> > tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE));
> > tcet->liobn = liobn;
> >
> > - snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn);
> > + tmp = g_strdup_printf("tce-table-%x", liobn);
> > object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL);
> > + g_free(tmp);
> >
> > object_property_set_bool(OBJECT(tcet), true, "realized", NULL);
> >
> >
> >
>
>
pgpBVpnXoSLD1.pgp
Description: OpenPGP digital signature
- Re: [Qemu-ppc] [Qemu-devel] [for-2.11 PATCH 01/26] spapr: move spapr_create_phb() to core machine code, (continued)
[Qemu-ppc] [for-2.11 PATCH 05/26] spapr_iommu: convert TCE table object to realize(), Greg Kurz, 2017/07/25
[Qemu-ppc] [for-2.11 PATCH 06/26] spapr_pci: parent the MSI memory region to the PHB, Greg Kurz, 2017/07/25