From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm: do not use device name as a format string Date: Mon, 7 Dec 2015 08:43:12 +0100 Message-ID: <20151207074312.GU10243@phenom.ffwll.local> References: <1447869498-13277-1-git-send-email-nicolas.iooss_linux@m4x.org> <5662B24E.4090202@m4x.org> <20151206093559.GT10243@phenom.ffwll.local> <56640B00.7040100@m4x.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <56640B00.7040100@m4x.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Nicolas Iooss Cc: Terje =?iso-8859-1?Q?Bergstr=F6m?= , Alison Wang , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org List-Id: linux-tegra@vger.kernel.org T24gU3VuLCBEZWMgMDYsIDIwMTUgYXQgMTE6MTY6MzJBTSArMDEwMCwgTmljb2xhcyBJb29zcyB3 cm90ZToKPiBPbiAxMi8wNi8yMDE1IDEwOjM1IEFNLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+ID4+ IE9uIDExLzE4LzIwMTUgMDY6NTggUE0sIE5pY29sYXMgSW9vc3Mgd3JvdGU6Cj4gPj4+IGRybV9k ZXZfc2V0X3VuaXF1ZSgpIGZvcm1hdHMgaXRzIHBhcmFtZXRlciB1c2luZyBrdmFzcHJpbnRmKCkg YnV0IG1hbnkKPiA+Pj4gb2YgaXRzIGNhbGxlcnMgZGlyZWN0bHkgcGFzcyBkZXZfbmFtZShkZXYp IGFzIHByaW50ZiBmb3JtYXQgc3RyaW5nLAo+ID4+PiB3aXRob3V0IGFueSBmb3JtYXQgcGFyYW1l dGVyLiAgVGhpcyBjYW4gY2F1c2Ugc29tZSBpc3N1ZXMgd2hlbiB0aGUKPiA+Pj4gZGV2aWNlIG5h bWUgY29udGFpbnMgJyUnIGNoYXJhY3RlcnMuCj4gPj4+Cj4gPj4+IFRvIGF2b2lkIGFueSBwb3Rl bnRpYWwgaXNzdWUsIGFsd2F5cyB1c2UgIiVzIiB3aGVuIHVzaW5nCj4gPj4+IGRybV9kZXZfc2V0 X3VuaXF1ZSgpIHdpdGggZGV2X25hbWUoKS4KPiA+IAo+ID4gTm90IHN1cmUgdGhpcyBpcyB3b3J0 aCBpdCByZWFsbHksIG5vcm1hbGx5IHBlb3BsZSBkb24ndCBwbGFjZSAlIGNoYXJhY3RlcnMKPiA+ IGludG8gdGhlaXIgZGV2aWNlIG5hbWVzLCBldmVyLiBBbmQgaWYgdGhleSBkbyBpdCdsbCBibG93 IHVwLiBUaGVyZSdzIGFsc28KPiA+IG5vIHNlY3VyaXR5IGlzc3VlIGhlcmUgc2luY2UgdXNlcnNw YWNlIGNhbid0IHNldCB0aGlzIG5hbWUuCj4gPiAKPiA+IElmIHRoZSBtYWludGFpbmVycyBvZiB0 aGUgYWZmZWN0ZWQgZHJpdmVycyBkb24ndCB3YW50IHRoaXMgSSB3b24ndCBtZXJnZQo+ID4gdGhp cyBwYXRjaC4KPiAKPiBBY3R1YWxseSBJIGhhZCB0aGUgc2FtZSBvcGluaW9uIGJlZm9yZSBJIGJl Z2FuIHRvIGFkZCBfX3ByaW50Zgo+IGF0dHJpYnV0ZXMgYW5kICIlcyIgaW4gc2V2ZXJhbCBwbGFj ZXMgaW4gdGhlIGtlcm5lbCB0byBtYWtlCj4gLVdmb3JtYXQtc2VjdXJpdHkgdXNlZnVsLiAgVGhp cyBsZWQgbWUgdG8gZGlzY292ZXIgc29tZSBmdW5ueSBpc3N1ZXMKPiBsaWtlIHRoZSBvbmUgZml4 ZWQgYnkgY29tbWl0IDM5NThiNzkyNjZiMSAoImNvbmZpZ2ZzOiBmaXgga2VybmVsCj4gaW5mb2xl YWsgdGhyb3VnaCB1c2VyLWNvbnRyb2xsZWQgZm9ybWF0IHN0cmluZyIsCj4gaHR0cHM6Ly9naXQu a2VybmVsLm9yZy9jZ2l0L2xpbnV4L2tlcm5lbC9naXQvdG9ydmFsZHMvbGludXguZ2l0L2NvbW1p dC8/aWQ9Mzk1OGI3OTI2NmIxNDcyOWVkZDYxZGFmOWRmYjg0ZGU0NWY0ZWM2ZAo+ICkuICBUaGUg cGF0Y2ggSSBzZW50IGlzIGluIGZhY3QgYSB2ZXJ5IHNtYWxsIHN0ZXAgdG93YXJkcyBtYWtpbmcK PiAtV2Zvcm1hdC1zZWN1cml0eSB1c2VmdWwgYWdhaW4gdG8gZGV0ZWN0ICJyZWFsIiBpc3N1ZXMu Cj4gCj4gT2YgY291cnNlLCBpZiB5b3UgZG8gbm90IGZlZWwgaXQgaXMgd29ydGggaXQgYW5kIGJl bGlldmUgdGhhdCBkZXZfbmFtZQo+IGlzIGZ1bGx5IGNvbnRyb2xsZWQgYnkgdHJ1c3RlZCBzb3Vy Y2VzIHdoaWNoIHdpbGwgbmV2ZXIgaW50cm9kdWNlIGFueSAlCj4gY2hhcmFjdGVyLCBJIHVuZGVy c3RhbmQgeW91ciB3aWxsIG9mIG5vdCBtZXJnaW5nIG15IHBhdGNoLgoKQWgsIHRoYXQncyB0aGUg Y29udGV4dCBJIHdhcyBtaXNzaW5nLCB0aGF0IHJlYWxseSBzaG91bGQgYmUgaW4gdGhlIGNvbW1p dAptZXNzYWdlLiBJZiB0aGlzIGlzIHBhcnQgb2YgYW4gb3ZlcmFsbCBlZmZvcnQgdG8gZW5hYmxl IHNvbWV0aGluZyB1c2VmdWwKaXQgbWFrZXMgbW9yZSBzZW5zZSB0byBnZXQgaXQgaW4uCgpPbiB0 aGUgcGF0Y2ggaXRzZWxmIGl0IGZlZWxzIHJhdGhlciBmdW5ueSB0byBkbyBhICIlcyIsIHN0cik7 IGNvbWJvLCBtYXliZQp3ZSBzaG91bGQgaGF2ZSBhIGRybV9kZXZfc2V0X3VuaXF1ZV9zdGF0aWMg aW5zdGVhZD8gSW5jbHVkaW5nIGtlcm5lbGRvYwpleHBsYWluaW5nIHdoeSB0aGVyZSdzIHRvby4K LURhbmllbAotLSAKRGFuaWVsIFZldHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9y YXRpb24KaHR0cDovL2Jsb2cuZmZ3bGwuY2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0 aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754784AbbLGHnz (ORCPT ); Mon, 7 Dec 2015 02:43:55 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:33675 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754572AbbLGHnR (ORCPT ); Mon, 7 Dec 2015 02:43:17 -0500 Date: Mon, 7 Dec 2015 08:43:12 +0100 From: Daniel Vetter To: Nicolas Iooss Cc: Boris Brezillon , David Airlie , Jianwei Wang , Alison Wang , Thierry Reding , Terje =?iso-8859-1?Q?Bergstr=F6m?= , dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm: do not use device name as a format string Message-ID: <20151207074312.GU10243@phenom.ffwll.local> Mail-Followup-To: Nicolas Iooss , Boris Brezillon , David Airlie , Jianwei Wang , Alison Wang , Thierry Reding , Terje =?iso-8859-1?Q?Bergstr=F6m?= , dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org References: <1447869498-13277-1-git-send-email-nicolas.iooss_linux@m4x.org> <5662B24E.4090202@m4x.org> <20151206093559.GT10243@phenom.ffwll.local> <56640B00.7040100@m4x.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56640B00.7040100@m4x.org> X-Operating-System: Linux phenom 4.1.0-2-amd64 User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Dec 06, 2015 at 11:16:32AM +0100, Nicolas Iooss wrote: > On 12/06/2015 10:35 AM, Daniel Vetter wrote: > >> On 11/18/2015 06:58 PM, Nicolas Iooss wrote: > >>> drm_dev_set_unique() formats its parameter using kvasprintf() but many > >>> of its callers directly pass dev_name(dev) as printf format string, > >>> without any format parameter. This can cause some issues when the > >>> device name contains '%' characters. > >>> > >>> To avoid any potential issue, always use "%s" when using > >>> drm_dev_set_unique() with dev_name(). > > > > Not sure this is worth it really, normally people don't place % characters > > into their device names, ever. And if they do it'll blow up. There's also > > no security issue here since userspace can't set this name. > > > > If the maintainers of the affected drivers don't want this I won't merge > > this patch. > > Actually I had the same opinion before I began to add __printf > attributes and "%s" in several places in the kernel to make > -Wformat-security useful. This led me to discover some funny issues > like the one fixed by commit 3958b79266b1 ("configfs: fix kernel > infoleak through user-controlled format string", > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3958b79266b14729edd61daf9dfb84de45f4ec6d > ). The patch I sent is in fact a very small step towards making > -Wformat-security useful again to detect "real" issues. > > Of course, if you do not feel it is worth it and believe that dev_name > is fully controlled by trusted sources which will never introduce any % > character, I understand your will of not merging my patch. Ah, that's the context I was missing, that really should be in the commit message. If this is part of an overall effort to enable something useful it makes more sense to get it in. On the patch itself it feels rather funny to do a "%s", str); combo, maybe we should have a drm_dev_set_unique_static instead? Including kerneldoc explaining why there's too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch