On Mon, 31 Jul 2017 07:11:45 -0300 Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi David,
>
> On 07/26/2017 12:58 AM, David Gibson wrote:
> > On Tue, Jul 25, 2017 at 07:58:53PM +0200, 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.
> >>
> >> Signed-off-by: Greg Kurz <gr...@kaod.org>
> >
> > Applied to ppc-for-2.11.
>
> Isn't it 2.10 material?
>
Hi Philippe,
Well... this patch doesn't fix any bug actually since the stack buffers
are large enough. It is more a question of coding style.
Something like below would have been more appropriate I guess:
"Building strings with g_strdup_printf() is a QEMU common practice."
No big deal.
Cheers,
--
Greg
> Regards,
>
> Phil.
>
> >
> >> ---
> >> hw/ppc/spapr_drc.c | 15 +++++++++------
> >> 1 file changed, 9 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> >> index 15bae5c216a9..e4e8383ec7b5 100644
> >> --- a/hw/ppc/spapr_drc.c
> >> +++ b/hw/ppc/spapr_drc.c
> >> @@ -488,7 +488,7 @@ static void realize(DeviceState *d, Error **errp)
> >> {
> >> sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
> >> Object *root_container;
> >> - char link_name[256];
> >> + gchar *link_name;
> >> gchar *child_name;
> >> Error *err = NULL;
> >>
> >> @@ -501,11 +501,12 @@ static void realize(DeviceState *d, Error **errp)
> >> * existing in the composition tree
> >> */
> >> root_container = container_get(object_get_root(),
> >> DRC_CONTAINER_PATH);
> >> - snprintf(link_name, sizeof(link_name), "%x", spapr_drc_index(drc));
> >> + link_name = g_strdup_printf("%x", spapr_drc_index(drc));
> >> child_name = object_get_canonical_path_component(OBJECT(drc));
> >> trace_spapr_drc_realize_child(spapr_drc_index(drc), child_name);
> >> object_property_add_alias(root_container, link_name,
> >> drc->owner, child_name, &err);
> >> + g_free(link_name);
> >> if (err) {
> >> error_report_err(err);
> >> object_unref(OBJECT(drc));
> >> @@ -521,13 +522,14 @@ static void unrealize(DeviceState *d, Error **errp)
> >> {
> >> sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
> >> Object *root_container;
> >> - char name[256];
> >> + gchar *name;
> >> Error *err = NULL;
> >>
> >> trace_spapr_drc_unrealize(spapr_drc_index(drc));
> >> root_container = container_get(object_get_root(),
> >> DRC_CONTAINER_PATH);
> >> - snprintf(name, sizeof(name), "%x", spapr_drc_index(drc));
> >> + name = g_strdup_printf("%x", spapr_drc_index(drc));
> >> object_property_del(root_container, name, &err);
> >> + g_free(name);
> >> if (err) {
> >> error_report_err(err);
> >> object_unref(OBJECT(drc));
> >> @@ -729,10 +731,11 @@ static const TypeInfo spapr_drc_lmb_info = {
> >> sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
> >> {
> >> Object *obj;
> >> - char name[256];
> >> + gchar *name;
> >>
> >> - snprintf(name, sizeof(name), "%s/%x", DRC_CONTAINER_PATH, index);
> >> + name = g_strdup_printf("%s/%x", DRC_CONTAINER_PATH, index);
> >> obj = object_resolve_path(name, NULL);
> >> + g_free(name);
> >>
> >> return !obj ? NULL : SPAPR_DR_CONNECTOR(obj);
> >> }
> >>
> >
pgpOCtSisCtu9.pgp
Description: OpenPGP digital signature
