|
From: | Liam Merwick |
Subject: | Re: [Qemu-devel] [PATCH 6/8] block: dump_qlist() may dereference a Null pointer |
Date: | Fri, 31 Aug 2018 14:27:36 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 30/08/18 19:41, Eric Blake wrote:
On 08/30/2018 10:47 AM, Liam Merwick wrote:A NULL 'list' passed into function dump_qlist() isn't correctly validated and can be passed to qlist_first() where it is dereferenced.But dump_qlist() is static, and it is easy to prove that it will never be called with a NULL 'list' parameter (it's lone caller did switch (qobject_type(obj)), and calls dump_qlist() only for QTYPE_QLIST, which implies that the qobject_to(QList, obj) will succeed and never be NULL).This could be resolved by checking if the list is NULL in dump_qlist() and returning immediately. However, the general case can be handled by adding a NULL arg check to to qlist_first() and qlist_next() and all the callers to those functions handle that cleanly.NACK. If anything, I'd be happier with: assert(list);
Thank works for me too. Regards, Liam
in dump_qlist() to shut up the lint checker, as we do not want to slow down the common case of qlist_first() for something that does not happen. That is, the null dereference in qlist_first() is a feature for detecting buggy code, and not something we need to change.Signed-off-by: Liam Merwick <address@hidden> Reviewed-by: Darren Kenny <address@hidden> Reviewed-by: Mark Kanda <address@hidden> --- include/qapi/qmp/qlist.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h index 8d2c32ca2863..1ec716e2eb9e 100644 --- a/include/qapi/qmp/qlist.h +++ b/include/qapi/qmp/qlist.h @@ -58,11 +58,17 @@ void qlist_destroy_obj(QObject *obj); static inline const QListEntry *qlist_first(const QList *qlist) { + if (!qlist) { + return NULL; + } return QTAILQ_FIRST(&qlist->head); } static inline const QListEntry *qlist_next(const QListEntry *entry) { + if (!entry) { + return NULL; + } return QTAILQ_NEXT(entry, next); }
[Prev in Thread] | Current Thread | [Next in Thread] |