From nobody Wed Feb 11 02:10:35 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1498140654387672.6531324223133; Thu, 22 Jun 2017 07:10:54 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5CC05142869; Thu, 22 Jun 2017 14:10:51 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E66875C7A5; Thu, 22 Jun 2017 14:10:50 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 9E1444E987; Thu, 22 Jun 2017 14:10:49 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v5ME2qNQ004457 for ; Thu, 22 Jun 2017 10:02:52 -0400 Received: by smtp.corp.redhat.com (Postfix) id 26DEA5D967; Thu, 22 Jun 2017 14:02:52 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-36.phx2.redhat.com [10.3.116.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id DB1425D96A for ; Thu, 22 Jun 2017 14:02:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5CC05142869 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 5CC05142869 From: John Ferlan To: libvir-list@redhat.com Date: Thu, 22 Jun 2017 10:02:32 -0400 Message-Id: <20170622140246.31777-3-jferlan@redhat.com> In-Reply-To: <20170622140246.31777-1-jferlan@redhat.com> References: <20170622140246.31777-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v3 02/16] util: Add safety net of checks to ensure valid object X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 22 Jun 2017 14:10:52 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The virObject logic "assumes" that whatever is passed to its API's would be some sort of virObjectPtr; however, if it is not then some really bad things can happen. So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock}, and virObjectIsClass and the virObject and virObjectLockable class consumers have been well behaved and code well tested. Soon there will be more consumers and one such consumer tripped over this during testing by passing a virHashTablePtr to virObjectIsClass which ends up calling virClassIsDerivedFrom using "obj->klass", which wasn't really a klass object causing one of those bad things to happen. To avoid the future possibility that a non virObject class memory was passed to some virObject* API, this patch adds two new checks - one to validate that the object has the 0xCAFExxxx value in obj->->u.s.magic and the other to ensure obj->u.s.magic doesn't "wrap" some day to 0xCAFF0000 if we ever get that many object classes. The check is also moved before the name VIR_STRDUP to avoid the extra VIR_FREE that would be required on the failure path. It is still left up to the caller to handle the failed API calls just as it would be if it passed a NULL opaque pointer anyobj. Signed-off-by: John Ferlan --- src/util/virobject.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/util/virobject.c b/src/util/virobject.c index ff9385b..443718b 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -47,14 +47,21 @@ struct _virClass { virObjectDisposeCallback dispose; }; =20 +#define VIR_OBJECT_NOTVALID(obj) (!obj || ((obj->u.s.magic & 0xCAFE0000) != =3D 0xCAFE0000)) + #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass) = \ do { = \ virObjectPtr obj =3D anyobj; = \ - if (!obj) = \ - VIR_WARN("Object cannot be NULL"); = \ - else = \ + if (VIR_OBJECT_NOTVALID(obj)) { = \ + if (!obj) = \ + VIR_WARN("Object cannot be NULL"); = \ + else = \ + VIR_WARN("Object %p has a bad magic number %X", = \ + obj, obj->u.s.magic); = \ + } else { = \ VIR_WARN("Object %p (%s) is not a %s instance", = \ anyobj, obj->klass->name, #objclass); = \ + } = \ } while (0) =20 static virClassPtr virObjectClass; @@ -153,9 +160,14 @@ virClassNew(virClassPtr parent, goto error; =20 klass->parent =3D parent; + klass->magic =3D virAtomicIntInc(&magicCounter); + if (klass->magic > 0xCAFEFFFF) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("too many object classes defined")); + goto error; + } if (VIR_STRDUP(klass->name, name) < 0) goto error; - klass->magic =3D virAtomicIntInc(&magicCounter); klass->objectSize =3D objectSize; klass->dispose =3D dispose; =20 @@ -272,7 +284,7 @@ virObjectUnref(void *anyobj) { virObjectPtr obj =3D anyobj; =20 - if (!obj) + if (VIR_OBJECT_NOTVALID(obj)) return false; =20 bool lastRef =3D virAtomicIntDecAndTest(&obj->u.s.refs); @@ -311,7 +323,7 @@ virObjectRef(void *anyobj) { virObjectPtr obj =3D anyobj; =20 - if (!obj) + if (VIR_OBJECT_NOTVALID(obj)) return NULL; virAtomicIntInc(&obj->u.s.refs); PROBE(OBJECT_REF, "obj=3D%p", obj); @@ -389,7 +401,7 @@ virObjectIsClass(void *anyobj, virClassPtr klass) { virObjectPtr obj =3D anyobj; - if (!obj) + if (VIR_OBJECT_NOTVALID(obj)) return false; =20 return virClassIsDerivedFrom(obj->klass, klass); --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list