| Message ID | 5aa4a6417b0f7acbfd7f4c21dca26293bc3d9348.1710282274.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/68] vdpa: add back vhost_vdpa_net_first_nc_vdpa | expand |
Hi Michael, Thanks a lot and aslo this one, I forgot to update the status :/, this patch has been merged (commit d470fd6acd60, "hw/display/macfb: Fix missing ERRP_GUARD() in macfb_nubus_realize()"). Best Regards, Zhao On Tue, Mar 12, 2024 at 06:27:16PM -0400, Michael S. Tsirkin wrote: > Date: Tue, 12 Mar 2024 18:27:16 -0400 > From: "Michael S. Tsirkin" <mst@redhat.com> > Subject: [PULL 34/68] hw/display/macfb: Fix missing ERRP_GUARD() in > macfb_nubus_realize() > X-Mailer: git-send-email 2.27.0.106.g8ac3dc51b1 > > From: Zhao Liu <zhao1.liu@intel.com> > > As the comment in qapi/error, dereferencing @errp requires > ERRP_GUARD(): > > * = Why, when and how to use ERRP_GUARD() = > * > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > * - It must not be dereferenced, because it may be null. > ... > * ERRP_GUARD() lifts these restrictions. > * > * To use ERRP_GUARD(), add it right at the beginning of the function. > * @errp can then be used without worrying about the argument being > * NULL or &error_fatal. > * > * Using it when it's not needed is safe, but please avoid cluttering > * the source with useless code. > > But in macfb_nubus_realize(), @errp is dereferenced without > ERRP_GUARD(): > > ndc->parent_realize(dev, errp); > if (*errp) { > return; > } > > Here we check *errp, because the ndc->parent_realize(), as a > DeviceClass.realize() callback, returns void. And since > macfb_nubus_realize(), also as a DeviceClass.realize(), doesn't get the > NULL @errp parameter, it hasn't triggered the bug that dereferencing the > NULL @errp. > > To follow the requirement of @errp, add missing ERRP_GUARD() in > macfb_nubus_realize(). > > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Message-Id: <20240223085653.1255438-3-zhao1.liu@linux.intel.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/display/macfb.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/display/macfb.c b/hw/display/macfb.c > index 418e99c8e1..1ace341a0f 100644 > --- a/hw/display/macfb.c > +++ b/hw/display/macfb.c > @@ -714,6 +714,7 @@ static void macfb_nubus_set_irq(void *opaque, int n, int level) > > static void macfb_nubus_realize(DeviceState *dev, Error **errp) > { > + ERRP_GUARD(); > NubusDevice *nd = NUBUS_DEVICE(dev); > MacfbNubusState *s = NUBUS_MACFB(dev); > MacfbNubusDeviceClass *ndc = NUBUS_MACFB_GET_CLASS(dev); > -- > MST >
On 13/3/24 03:23, Zhao Liu wrote: > Hi Michael, > > Thanks a lot and aslo this one, I forgot to update the status :/, > this patch has been merged (commit d470fd6acd60, "hw/display/macfb: > Fix missing ERRP_GUARD() in macfb_nubus_realize()"). Normally git should cope with this; if so, no need to repost :) > > Best Regards, > Zhao
diff --git a/hw/display/macfb.c b/hw/display/macfb.c index 418e99c8e1..1ace341a0f 100644 --- a/hw/display/macfb.c +++ b/hw/display/macfb.c @@ -714,6 +714,7 @@ static void macfb_nubus_set_irq(void *opaque, int n, int level) static void macfb_nubus_realize(DeviceState *dev, Error **errp) { + ERRP_GUARD(); NubusDevice *nd = NUBUS_DEVICE(dev); MacfbNubusState *s = NUBUS_MACFB(dev); MacfbNubusDeviceClass *ndc = NUBUS_MACFB_GET_CLASS(dev);