From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31549 invoked by alias); 7 Feb 2007 03:24:28 -0000 Received: (qmail 31538 invoked by uid 22791); 7 Feb 2007 03:24:28 -0000 X-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mga02.intel.com (HELO mga02.intel.com) (134.134.136.20) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 07 Feb 2007 03:24:20 +0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by mga02.intel.com with ESMTP; 06 Feb 2007 19:24:18 -0800 Received: from fmsmsx334.amr.corp.intel.com ([132.233.42.1]) by orsmga001.jf.intel.com with ESMTP; 06 Feb 2007 19:24:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: i="4.13,292,1167638400"; d="scan'208"; a="193630207:sNHT19159112" Received: from scsmsx411.amr.corp.intel.com ([10.3.90.30]) by fmsmsx334.amr.corp.intel.com with Microsoft SMTPSVC(6.0.3790.1830); Tue, 6 Feb 2007 19:24:17 -0800 Received: from scsmsx413.amr.corp.intel.com ([10.3.90.32]) by scsmsx411.amr.corp.intel.com with Microsoft SMTPSVC(6.0.3790.1830); Tue, 6 Feb 2007 19:24:18 -0800 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Subject: Tapset dereferencing audit X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 07 Feb 2007 03:24:00 -0000 Message-ID: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: Tapset dereferencing audit Thread-Index: AcdKZ3LoM9avmd/7QKivxRyEnUDB0A== From: "Stone, Joshua I" To: X-OriginalArrivalTime: 07 Feb 2007 03:24:18.0410 (UTC) FILETIME=[738DACA0:01C74A67] X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2007-q1/txt/msg00296.txt.bz2 Hi all, I just made a rather large checkin on the tapsets, so I figured it was worth an announcment. My goal in reviewing the tapsets was to make sure that everywhere we dereference potentially-invalid pointers, we use the appropriate macros to protect against faults. We've had deref(), store_deref(), and deref_string() for a while for this purpose, and recently I added kread() and kwrite() for more convenience (they infer the typecast and size from the pointer type). I have a few observations: * Tapset functions written in embedded-C should *not* use a return statement. Yes, it works today if you do, but we might not guarantee that forever. Don't do it. If you really need a way to end execution of the function, use a goto to the end of your function body. * Some dereferences are still left unchecked. I left these in places that make function calls, or use complicated macros. We'll either need to manually reproduce the functionality of those calls with our protection in place, or use some other method to catch any potential faults. I marked these with FIXME wherever I saw a potential problem. * In many places the tapset functions were checking for NULL pointers and returning a predetermined value, with no error. I tried to preserve this as much as possible. However, I suspect that many of those NULL-checks were just a feeble attempt to avoid bad pointers -- it's important to realize that non-NULL might still be bad! So, my call to the various tapset owners: please review whether each of your NULL-checks is necessary in light of my new changes. There are probably some legitimate cases where a pointer might be NULL but shouldn't signal an error. In all other cases, just remove the check and let kread() throw an error to the user. As far as the testsuite can show, I didn't introduce any new regressions. But of course, everyone please run tests on your own machines so we can make sure. Thanks, Josh