From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935701AbcKWLX1 (ORCPT ); Wed, 23 Nov 2016 06:23:27 -0500 Received: from foss.arm.com ([217.140.101.70]:49370 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751010AbcKWLX0 (ORCPT ); Wed, 23 Nov 2016 06:23:26 -0500 Date: Wed, 23 Nov 2016 11:23:23 +0000 From: Liviu Dudau To: Jani Nikula Cc: Eric Engestrom , Daniel Vetter , LKML , DRI devel Subject: Re: [PATCH v2] drm: check for NULL parameter in exported drm_get_format_name() function. Message-ID: <20161123112323.GX1005@e106497-lin.cambridge.arm.com> References: <20161123105213.27674-1-Liviu.Dudau@arm.com> <87vavewjew.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87vavewjew.fsf@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 23, 2016 at 01:00:07PM +0200, Jani Nikula wrote: > On Wed, 23 Nov 2016, Liviu Dudau wrote: > > drm_get_format_name() de-references the buf parameter without checking > > if the pointer was not NULL. Given that the function is EXPORT-ed, lets > > sanitise the parameters before proceeding. > > > > v2: Use BUG_ON() to annoy users that did not pass valid parameters to function. > > > > Fixes: b3c11ac267d461d3d5 ("drm: move allocation out of drm_get_format_name()) > > Cc: Eric Engestrom > > Cc: Rob Clark > > Cc: Jani Nikula > > Cc: Daniel Vetter > > > > Signed-off-by: Liviu Dudau > > --- > > I still think sanity checking the parameters of an exported function is worth > > doing, even if the way one triggers the NULL pointer crash is priviledged. Not > > a big fan of the verbosity of BUG_ON() and would rather silently reject NULL buf > > pointer, but that is a matter of taste. > > There really is no meaningful difference between doing BUG_ON(!bug) > vs. just letting buf->str oops. The kernel is full of functions that > expect sensible pointers, and I don't see why this one in particular > should be so special to warrant a BUG_ON(). Agree. That is why I prefer v1 where I return immediately on NULL pointers. Best regards, Liviu > > BR, > Jani. > > > > > > > drivers/gpu/drm/drm_fourcc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > index 90d2cc8..6d80239 100644 > > --- a/drivers/gpu/drm/drm_fourcc.c > > +++ b/drivers/gpu/drm/drm_fourcc.c > > @@ -85,6 +85,8 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format); > > */ > > const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf) > > { > > + BUG_ON(!buf); > > + > > snprintf(buf->str, sizeof(buf->str), > > "%c%c%c%c %s-endian (0x%08x)", > > printable_char(format & 0xff), > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH v2] drm: check for NULL parameter in exported drm_get_format_name() function. Date: Wed, 23 Nov 2016 11:23:23 +0000 Message-ID: <20161123112323.GX1005@e106497-lin.cambridge.arm.com> References: <20161123105213.27674-1-Liviu.Dudau@arm.com> <87vavewjew.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by gabe.freedesktop.org (Postfix) with ESMTP id 895B86E7DD for ; Wed, 23 Nov 2016 11:23:25 +0000 (UTC) Content-Disposition: inline In-Reply-To: <87vavewjew.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jani Nikula Cc: Daniel Vetter , Eric Engestrom , LKML , DRI devel List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCBOb3YgMjMsIDIwMTYgYXQgMDE6MDA6MDdQTSArMDIwMCwgSmFuaSBOaWt1bGEgd3Jv dGU6Cj4gT24gV2VkLCAyMyBOb3YgMjAxNiwgTGl2aXUgRHVkYXUgPExpdml1LkR1ZGF1QGFybS5j b20+IHdyb3RlOgo+ID4gZHJtX2dldF9mb3JtYXRfbmFtZSgpIGRlLXJlZmVyZW5jZXMgdGhlIGJ1 ZiBwYXJhbWV0ZXIgd2l0aG91dCBjaGVja2luZwo+ID4gaWYgdGhlIHBvaW50ZXIgd2FzIG5vdCBO VUxMLiBHaXZlbiB0aGF0IHRoZSBmdW5jdGlvbiBpcyBFWFBPUlQtZWQsIGxldHMKPiA+IHNhbml0 aXNlIHRoZSBwYXJhbWV0ZXJzIGJlZm9yZSBwcm9jZWVkaW5nLgo+ID4KPiA+IHYyOiBVc2UgQlVH X09OKCkgdG8gYW5ub3kgdXNlcnMgdGhhdCBkaWQgbm90IHBhc3MgdmFsaWQgcGFyYW1ldGVycyB0 byBmdW5jdGlvbi4KPiA+Cj4gPiBGaXhlczogYjNjMTFhYzI2N2Q0NjFkM2Q1ICgiZHJtOiBtb3Zl IGFsbG9jYXRpb24gb3V0IG9mIGRybV9nZXRfZm9ybWF0X25hbWUoKSkKPiA+IENjOiBFcmljIEVu Z2VzdHJvbSA8ZXJpY0Blbmdlc3Ryb20uY2g+Cj4gPiBDYzogUm9iIENsYXJrIDxyb2JkY2xhcmtA Z21haWwuY29tPgo+ID4gQ2M6IEphbmkgTmlrdWxhIDxqYW5pLm5pa3VsYUBpbnRlbC5jb20+Cj4g PiBDYzogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPiA+Cj4gPiBTaWdu ZWQtb2ZmLWJ5OiBMaXZpdSBEdWRhdSA8TGl2aXUuRHVkYXVAYXJtLmNvbT4KPiA+IC0tLQo+ID4g SSBzdGlsbCB0aGluayBzYW5pdHkgY2hlY2tpbmcgdGhlIHBhcmFtZXRlcnMgb2YgYW4gZXhwb3J0 ZWQgZnVuY3Rpb24gaXMgd29ydGgKPiA+IGRvaW5nLCBldmVuIGlmIHRoZSB3YXkgb25lIHRyaWdn ZXJzIHRoZSBOVUxMIHBvaW50ZXIgY3Jhc2ggaXMgcHJpdmlsZWRnZWQuIE5vdAo+ID4gYSBiaWcg ZmFuIG9mIHRoZSB2ZXJib3NpdHkgb2YgQlVHX09OKCkgYW5kIHdvdWxkIHJhdGhlciBzaWxlbnRs eSByZWplY3QgTlVMTCBidWYKPiA+IHBvaW50ZXIsIGJ1dCB0aGF0IGlzIGEgbWF0dGVyIG9mIHRh c3RlLgo+IAo+IFRoZXJlIHJlYWxseSBpcyBubyBtZWFuaW5nZnVsIGRpZmZlcmVuY2UgYmV0d2Vl biBkb2luZyBCVUdfT04oIWJ1ZykKPiB2cy4ganVzdCBsZXR0aW5nIGJ1Zi0+c3RyIG9vcHMuIFRo ZSBrZXJuZWwgaXMgZnVsbCBvZiBmdW5jdGlvbnMgdGhhdAo+IGV4cGVjdCBzZW5zaWJsZSBwb2lu dGVycywgYW5kIEkgZG9uJ3Qgc2VlIHdoeSB0aGlzIG9uZSBpbiBwYXJ0aWN1bGFyCj4gc2hvdWxk IGJlIHNvIHNwZWNpYWwgdG8gd2FycmFudCBhIEJVR19PTigpLgoKQWdyZWUuIFRoYXQgaXMgd2h5 IEkgcHJlZmVyIHYxIHdoZXJlIEkgcmV0dXJuIGltbWVkaWF0ZWx5IG9uIE5VTEwgcG9pbnRlcnMu CgpCZXN0IHJlZ2FyZHMsCkxpdml1Cgo+IAo+IEJSLAo+IEphbmkuCj4gCj4gPgo+ID4KPiA+ICBk cml2ZXJzL2dwdS9kcm0vZHJtX2ZvdXJjYy5jIHwgMiArKwo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAy IGluc2VydGlvbnMoKykKPiA+Cj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9m b3VyY2MuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZm91cmNjLmMKPiA+IGluZGV4IDkwZDJjYzgu LjZkODAyMzkgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vZHJtX2ZvdXJjYy5jCj4g PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2ZvdXJjYy5jCj4gPiBAQCAtODUsNiArODUsOCBA QCBFWFBPUlRfU1lNQk9MKGRybV9tb2RlX2xlZ2FjeV9mYl9mb3JtYXQpOwo+ID4gICAqLwo+ID4g IGNvbnN0IGNoYXIgKmRybV9nZXRfZm9ybWF0X25hbWUodWludDMyX3QgZm9ybWF0LCBzdHJ1Y3Qg ZHJtX2Zvcm1hdF9uYW1lX2J1ZiAqYnVmKQo+ID4gIHsKPiA+ICsJQlVHX09OKCFidWYpOwo+ID4g Kwo+ID4gIAlzbnByaW50ZihidWYtPnN0ciwgc2l6ZW9mKGJ1Zi0+c3RyKSwKPiA+ICAJCSAiJWMl YyVjJWMgJXMtZW5kaWFuICgweCUwOHgpIiwKPiA+ICAJCSBwcmludGFibGVfY2hhcihmb3JtYXQg JiAweGZmKSwKPiAKPiAtLSAKPiBKYW5pIE5pa3VsYSwgSW50ZWwgT3BlbiBTb3VyY2UgVGVjaG5v bG9neSBDZW50ZXIKPiBfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwo+IGRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKPiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCj4gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9k cmktZGV2ZWwKCi0tIAo9PT09PT09PT09PT09PT09PT09PQp8IEkgd291bGQgbGlrZSB0byB8Cnwg Zml4IHRoZSB3b3JsZCwgIHwKfCBidXQgdGhleSdyZSBub3QgfAp8IGdpdmluZyBtZSB0aGUgICB8 CiBcIHNvdXJjZSBjb2RlISAgLwogIC0tLS0tLS0tLS0tLS0tLQogICAgwq9cXyjjg4QpXy/Crwpf X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwg bWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK