From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="IVIY59PY" X-Greylist: delayed 62 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Wed, 29 Nov 2023 03:41:49 PST Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D8703C4; Wed, 29 Nov 2023 03:41:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701258110; x=1732794110; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=w91TqZU/Sz95TBAtE+lLgPRTyMnYI3/7ak+ZXvIxwqY=; b=IVIY59PY2nmI34Vxl+3e70xlIK4bnrx6q7yyTPt0My8bGdNinYgSBbIy OElb5mjET9zVtpoTq2JdYV8ARKdl/9dOPQVFUD88SimIq1a6D4swPHpAB ai5mXguu4Ry/0T3sdLLLw89UzK4FJhAB98+lufdce4aTwJn5uANQGFGpw olcagkl67s51Jz3KcyksvcIxoJcCWmgS1w3g7yST8g2X/lhyj7Vs6e/L9 0IcME2kDIFWl2vVo0W3LRa0GBFnp9CFLTyT1hwT0gtMaJPZOtG1PS971k /bNjkOxwyKglRkseeGlrVr5gvIGNUyI4unFHw+5IC/7u8LHLMTOMasbNL w==; X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="35035" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="35035" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="942291528" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="942291528" Received: from dstavrak-mobl.ger.corp.intel.com (HELO localhost) ([10.252.60.61]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:40 -0800 From: Jani Nikula To: Maxime Ripard Cc: Ville =?utf-8?B?U3lyasOkbMOk?= , Thomas Zimmermann , Emma Anholt , Jonathan Corbet , linux-kernel@vger.kernel.org, Samuel Holland , Sandy Huang , Jernej Skrabec , linux-doc@vger.kernel.org, Hans Verkuil , linux-rockchip@lists.infradead.org, Chen-Yu Tsai , dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 05/45] drm/connector: Check drm_connector_init pointers arguments In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20231128-kms-hdmi-connector-state-v4-0-c7602158306e@kernel.org> <20231128-kms-hdmi-connector-state-v4-5-c7602158306e@kernel.org> <87h6l66nth.fsf@intel.com> <2mnodqvu2oo674vspiy4gxhglu3it5cq47acx5itnbwevgc4cf@c7h2bvnx3m2n> <8734wo7vbx.fsf@intel.com> Date: Wed, 29 Nov 2023 13:40:38 +0200 Message-ID: <87ttp46b49.fsf@intel.com> Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Wed, 29 Nov 2023, Maxime Ripard wrote: > On Wed, Nov 29, 2023 at 11:38:42AM +0200, Jani Nikula wrote: >> On Wed, 29 Nov 2023, Maxime Ripard wrote: >> > Hi Ville, >> > >> > On Tue, Nov 28, 2023 at 03:49:08PM +0200, Ville Syrj=C3=A4l=C3=A4 wrot= e: >> >> On Tue, Nov 28, 2023 at 02:29:40PM +0100, Maxime Ripard wrote: >> >> > On Tue, Nov 28, 2023 at 02:54:02PM +0200, Jani Nikula wrote: >> >> > > On Tue, 28 Nov 2023, Maxime Ripard wrote: >> >> > > > All the drm_connector_init variants take at least a pointer to = the >> >> > > > device, connector and hooks implementation. >> >> > > > >> >> > > > However, none of them check their value before dereferencing th= ose >> >> > > > pointers which can lead to a NULL-pointer dereference if the au= thor >> >> > > > isn't careful. >> >> > >=20 >> >> > > Arguably oopsing on the spot is preferrable when this can't be ca= used by >> >> > > user input. It's always a mistake that should be caught early dur= ing >> >> > > development. >> >> > >=20 >> >> > > Not everyone checks the return value of drm_connector_init and fr= iends, >> >> > > so those cases will lead to more mysterious bugs later. And proba= bly >> >> > > oopses as well. >> >> >=20 >> >> > So maybe we can do both then, with something like >> >> >=20 >> >> > if (WARN_ON(!dev)) >> >> > return -EINVAL >> >> >=20 >> >> > if (drm_WARN_ON(dev, !connector || !funcs)) >> >> > return -EINVAL; >> >> >=20 >> >> > I'd still like to check for this, so we can have proper testing, an= d we >> >> > already check for those pointers in some places (like funcs in >> >> > drm_connector_init), so if we don't cover everything we're inconsis= tent. >> >>=20 >> >> People will invariably cargo-cult this kind of stuff absolutely >> >> everywhere and then all your functions will have tons of dead >> >> code to check their arguments. >> > >> > And that's a bad thing because... ? >> > >> > Also, are you really saying that checking that your arguments make sen= se >> > is cargo-cult? >>=20 >> It's a powerful thing to be able to assume a NULL argument is always a >> fatal programming error on the caller's side, and should oops and get >> caught immediately. It's an assertion. > > Yeah, but we're not really doing that either. We have no explicit > assertion anywhere. We take a pointer in, and just hope that it will be > dereferenced later on and that the kernel will crash. The pointer to the > functions especially is only deferenced very later on. > > And assertions might be powerful, but being able to notice errors and > debug them is too. A panic takes away basically any remote access to > debug. If you don't have a console, you're done. > >> We're not talking about user input or anything like that here. >>=20 >> If you start checking for things that can't happen, and return errors >> for them, you start gracefully handling things that don't have anything >> graceful about them. > > But there's nothing graceful to do here: you just return from your probe > function that you couldn't probe and that's it. Just like you do when > you can't map your registers, or get your interrupt, or register into > any framework (including drm_dev_register that pretty much every driver > handles properly if it returns an error, without being graceful about > it). Those are all dynamic things that can fail. Quite different from passing NULL dev, connector, or funcs to drm_connector_init() and friends. I think it's wrong to set the example that everything needs to be checked, everything needs to return an error, every call needs to check for error return, all the time, everywhere. People absolutely will cargo cult that, and that's what Ville is referring to. If you pass NULL dev, connector, or funcs to drm_connector_init() I think you absolutely deserve to get an oops. For dev, you could possibly not have reached the function with NULL dev. (And __drm_connector_init() has dev->mode_config before the check, so you'll get a static analyzer warning about dereference before the check.) If you have NULL connector, you didn't check for allocation failure earlier. If you have NULL funcs, you just passed NULL, because it's generally supposed to be a pointer to a static const struct. >> Having such checks in place trains people to think they *may* happen. > > In most cases, kmalloc can't fail. We seem to have a very different > policy towards it. Again, dynamic in nature and can fail. >> While it should fail fast and loud at the developer's first smoke test, >> and get fixed then and there. > > Returning an error + a warning also qualifies for "fail fast and loud". > But keeps the system alive for someone to notice in any case. But where do you draw the line? If we keep adding these checks to things that actually can't happen, we teach developers we need to check for impossible things. And we teach them not to trust anything. I scroll down the file and reach drm_connector_attach_edid_property(). Should we NULL check connector? Should we change the function to int and return a value? Should the caller check the value? Then there's drm_connector_attach_encoder(). And drm_connector_has_possible_encoder(). And so on and so forth. Where do you draw the line? BR, Jani. --=20 Jani Nikula, Intel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3997BC4167B for ; Wed, 29 Nov 2023 11:40:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=YvdIK/fDDRnX3NzTcZM+itTjwWTz8HHpvRm+rprV904=; b=RPyBjv4QD6hm+f aC56VtOjD8xsItE3qu/Fyjgd3TWK30Hank0rGRBGGS3Y35R7TL1qcPY6NjFNy/Bq5JalQLWBf07ha 5fGZeVsKBqZrupRGUV+pVHAFXaOCSX8fOCcfiDJH6f/+g+AgU/9OM09Qyi1jgwyQCrKkMu0ovr3tk dPqHtzu4RvAQyAkeicnzmkyucEdd2W99DLq6mzwnXt0p4my1ltJbKZVlnZ7vljakbLfyYQM+ghK2q 7nSI/RtAlNVzv5iFotTtrjgNl9+ag4Q3cnvoqT2h0fi/xCsGXYcmOoG3saYnYTtgeTecPik2mgCik YqQJ7/FQkjPqE7l7IQlg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8Iw0-0089qK-0o; Wed, 29 Nov 2023 11:40:52 +0000 Received: from mgamail.intel.com ([198.175.65.11]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r8Ivw-0089pm-2s; Wed, 29 Nov 2023 11:40:50 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701258049; x=1732794049; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=w91TqZU/Sz95TBAtE+lLgPRTyMnYI3/7ak+ZXvIxwqY=; b=CeWDV1pgm/8AvvRPX7R6ba9TLeyFX/w6u3MSVW0tp/iZRwXRKt5xvLMt NhHCNQ3F38HRkWOYMVsA71Iz736zA4cPMIdX0W7fxzLWhmbX8por6aNF8 BP+pSevsMMNHsPim+Og1qVWNqHvTR7sz2wfOZeeFtqL+qHg4rs/uMhphC evTSFy5X2iBodMO0u5XppzfoFLM0CBQeN/mMsaUcgYZvfk9RlGh8+IBu0 HeRbtSYdeCr1tgtMvpqxSX8pGutQip75GQ6Td0kK4nXmmGOKou4Xwpg7o Gl5y7qpJga0zgmJUh5XL1EAdXFNjJygjNZntVyHreXwnLlKrVNuSJVNzi Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="35043" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="35043" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="942291528" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="942291528" Received: from dstavrak-mobl.ger.corp.intel.com (HELO localhost) ([10.252.60.61]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:40 -0800 From: Jani Nikula To: Maxime Ripard Cc: Ville =?utf-8?B?U3lyasOkbMOk?= , Thomas Zimmermann , Emma Anholt , Jonathan Corbet , linux-kernel@vger.kernel.org, Samuel Holland , Sandy Huang , Jernej Skrabec , linux-doc@vger.kernel.org, Hans Verkuil , linux-rockchip@lists.infradead.org, Chen-Yu Tsai , dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 05/45] drm/connector: Check drm_connector_init pointers arguments In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20231128-kms-hdmi-connector-state-v4-0-c7602158306e@kernel.org> <20231128-kms-hdmi-connector-state-v4-5-c7602158306e@kernel.org> <87h6l66nth.fsf@intel.com> <2mnodqvu2oo674vspiy4gxhglu3it5cq47acx5itnbwevgc4cf@c7h2bvnx3m2n> <8734wo7vbx.fsf@intel.com> Date: Wed, 29 Nov 2023 13:40:38 +0200 Message-ID: <87ttp46b49.fsf@intel.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231129_034049_002406_02BCDFF0 X-CRM114-Status: GOOD ( 41.23 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org T24gV2VkLCAyOSBOb3YgMjAyMywgTWF4aW1lIFJpcGFyZCA8bXJpcGFyZEBrZXJuZWwub3JnPiB3 cm90ZToKPiBPbiBXZWQsIE5vdiAyOSwgMjAyMyBhdCAxMTozODo0MkFNICswMjAwLCBKYW5pIE5p a3VsYSB3cm90ZToKPj4gT24gV2VkLCAyOSBOb3YgMjAyMywgTWF4aW1lIFJpcGFyZCA8bXJpcGFy ZEBrZXJuZWwub3JnPiB3cm90ZToKPj4gPiBIaSBWaWxsZSwKPj4gPgo+PiA+IE9uIFR1ZSwgTm92 IDI4LCAyMDIzIGF0IDAzOjQ5OjA4UE0gKzAyMDAsIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPj4g Pj4gT24gVHVlLCBOb3YgMjgsIDIwMjMgYXQgMDI6Mjk6NDBQTSArMDEwMCwgTWF4aW1lIFJpcGFy ZCB3cm90ZToKPj4gPj4gPiBPbiBUdWUsIE5vdiAyOCwgMjAyMyBhdCAwMjo1NDowMlBNICswMjAw LCBKYW5pIE5pa3VsYSB3cm90ZToKPj4gPj4gPiA+IE9uIFR1ZSwgMjggTm92IDIwMjMsIE1heGlt ZSBSaXBhcmQgPG1yaXBhcmRAa2VybmVsLm9yZz4gd3JvdGU6Cj4+ID4+ID4gPiA+IEFsbCB0aGUg ZHJtX2Nvbm5lY3Rvcl9pbml0IHZhcmlhbnRzIHRha2UgYXQgbGVhc3QgYSBwb2ludGVyIHRvIHRo ZQo+PiA+PiA+ID4gPiBkZXZpY2UsIGNvbm5lY3RvciBhbmQgaG9va3MgaW1wbGVtZW50YXRpb24u Cj4+ID4+ID4gPiA+Cj4+ID4+ID4gPiA+IEhvd2V2ZXIsIG5vbmUgb2YgdGhlbSBjaGVjayB0aGVp ciB2YWx1ZSBiZWZvcmUgZGVyZWZlcmVuY2luZyB0aG9zZQo+PiA+PiA+ID4gPiBwb2ludGVycyB3 aGljaCBjYW4gbGVhZCB0byBhIE5VTEwtcG9pbnRlciBkZXJlZmVyZW5jZSBpZiB0aGUgYXV0aG9y Cj4+ID4+ID4gPiA+IGlzbid0IGNhcmVmdWwuCj4+ID4+ID4gPiAKPj4gPj4gPiA+IEFyZ3VhYmx5 IG9vcHNpbmcgb24gdGhlIHNwb3QgaXMgcHJlZmVycmFibGUgd2hlbiB0aGlzIGNhbid0IGJlIGNh dXNlZCBieQo+PiA+PiA+ID4gdXNlciBpbnB1dC4gSXQncyBhbHdheXMgYSBtaXN0YWtlIHRoYXQg c2hvdWxkIGJlIGNhdWdodCBlYXJseSBkdXJpbmcKPj4gPj4gPiA+IGRldmVsb3BtZW50Lgo+PiA+ PiA+ID4gCj4+ID4+ID4gPiBOb3QgZXZlcnlvbmUgY2hlY2tzIHRoZSByZXR1cm4gdmFsdWUgb2Yg ZHJtX2Nvbm5lY3Rvcl9pbml0IGFuZCBmcmllbmRzLAo+PiA+PiA+ID4gc28gdGhvc2UgY2FzZXMg d2lsbCBsZWFkIHRvIG1vcmUgbXlzdGVyaW91cyBidWdzIGxhdGVyLiBBbmQgcHJvYmFibHkKPj4g Pj4gPiA+IG9vcHNlcyBhcyB3ZWxsLgo+PiA+PiA+IAo+PiA+PiA+IFNvIG1heWJlIHdlIGNhbiBk byBib3RoIHRoZW4sIHdpdGggc29tZXRoaW5nIGxpa2UKPj4gPj4gPiAKPj4gPj4gPiBpZiAoV0FS Tl9PTighZGV2KSkKPj4gPj4gPiAgICByZXR1cm4gLUVJTlZBTAo+PiA+PiA+IAo+PiA+PiA+IGlm IChkcm1fV0FSTl9PTihkZXYsICFjb25uZWN0b3IgfHwgIWZ1bmNzKSkKPj4gPj4gPiAgICByZXR1 cm4gLUVJTlZBTDsKPj4gPj4gPiAKPj4gPj4gPiBJJ2Qgc3RpbGwgbGlrZSB0byBjaGVjayBmb3Ig dGhpcywgc28gd2UgY2FuIGhhdmUgcHJvcGVyIHRlc3RpbmcsIGFuZCB3ZQo+PiA+PiA+IGFscmVh ZHkgY2hlY2sgZm9yIHRob3NlIHBvaW50ZXJzIGluIHNvbWUgcGxhY2VzIChsaWtlIGZ1bmNzIGlu Cj4+ID4+ID4gZHJtX2Nvbm5lY3Rvcl9pbml0KSwgc28gaWYgd2UgZG9uJ3QgY292ZXIgZXZlcnl0 aGluZyB3ZSdyZSBpbmNvbnNpc3RlbnQuCj4+ID4+IAo+PiA+PiBQZW9wbGUgd2lsbCBpbnZhcmlh Ymx5IGNhcmdvLWN1bHQgdGhpcyBraW5kIG9mIHN0dWZmIGFic29sdXRlbHkKPj4gPj4gZXZlcnl3 aGVyZSBhbmQgdGhlbiBhbGwgeW91ciBmdW5jdGlvbnMgd2lsbCBoYXZlIHRvbnMgb2YgZGVhZAo+ PiA+PiBjb2RlIHRvIGNoZWNrIHRoZWlyIGFyZ3VtZW50cy4KPj4gPgo+PiA+IEFuZCB0aGF0J3Mg YSBiYWQgdGhpbmcgYmVjYXVzZS4uLiA/Cj4+ID4KPj4gPiBBbHNvLCBhcmUgeW91IHJlYWxseSBz YXlpbmcgdGhhdCBjaGVja2luZyB0aGF0IHlvdXIgYXJndW1lbnRzIG1ha2Ugc2Vuc2UKPj4gPiBp cyBjYXJnby1jdWx0Pwo+PiAKPj4gSXQncyBhIHBvd2VyZnVsIHRoaW5nIHRvIGJlIGFibGUgdG8g YXNzdW1lIGEgTlVMTCBhcmd1bWVudCBpcyBhbHdheXMgYQo+PiBmYXRhbCBwcm9ncmFtbWluZyBl cnJvciBvbiB0aGUgY2FsbGVyJ3Mgc2lkZSwgYW5kIHNob3VsZCBvb3BzIGFuZCBnZXQKPj4gY2F1 Z2h0IGltbWVkaWF0ZWx5LiBJdCdzIGFuIGFzc2VydGlvbi4KPgo+IFllYWgsIGJ1dCB3ZSdyZSBu b3QgcmVhbGx5IGRvaW5nIHRoYXQgZWl0aGVyLiBXZSBoYXZlIG5vIGV4cGxpY2l0Cj4gYXNzZXJ0 aW9uIGFueXdoZXJlLiBXZSB0YWtlIGEgcG9pbnRlciBpbiwgYW5kIGp1c3QgaG9wZSB0aGF0IGl0 IHdpbGwgYmUKPiBkZXJlZmVyZW5jZWQgbGF0ZXIgb24gYW5kIHRoYXQgdGhlIGtlcm5lbCB3aWxs IGNyYXNoLiBUaGUgcG9pbnRlciB0byB0aGUKPiBmdW5jdGlvbnMgZXNwZWNpYWxseSBpcyBvbmx5 IGRlZmVyZW5jZWQgdmVyeSBsYXRlciBvbi4KPgo+IEFuZCBhc3NlcnRpb25zIG1pZ2h0IGJlIHBv d2VyZnVsLCBidXQgYmVpbmcgYWJsZSB0byBub3RpY2UgZXJyb3JzIGFuZAo+IGRlYnVnIHRoZW0g aXMgdG9vLiBBIHBhbmljIHRha2VzIGF3YXkgYmFzaWNhbGx5IGFueSByZW1vdGUgYWNjZXNzIHRv Cj4gZGVidWcuIElmIHlvdSBkb24ndCBoYXZlIGEgY29uc29sZSwgeW91J3JlIGRvbmUuCj4KPj4g V2UncmUgbm90IHRhbGtpbmcgYWJvdXQgdXNlciBpbnB1dCBvciBhbnl0aGluZyBsaWtlIHRoYXQg aGVyZS4KPj4gCj4+IElmIHlvdSBzdGFydCBjaGVja2luZyBmb3IgdGhpbmdzIHRoYXQgY2FuJ3Qg aGFwcGVuLCBhbmQgcmV0dXJuIGVycm9ycwo+PiBmb3IgdGhlbSwgeW91IHN0YXJ0IGdyYWNlZnVs bHkgaGFuZGxpbmcgdGhpbmdzIHRoYXQgZG9uJ3QgaGF2ZSBhbnl0aGluZwo+PiBncmFjZWZ1bCBh Ym91dCB0aGVtLgo+Cj4gQnV0IHRoZXJlJ3Mgbm90aGluZyBncmFjZWZ1bCB0byBkbyBoZXJlOiB5 b3UganVzdCByZXR1cm4gZnJvbSB5b3VyIHByb2JlCj4gZnVuY3Rpb24gdGhhdCB5b3UgY291bGRu J3QgcHJvYmUgYW5kIHRoYXQncyBpdC4gSnVzdCBsaWtlIHlvdSBkbyB3aGVuCj4geW91IGNhbid0 IG1hcCB5b3VyIHJlZ2lzdGVycywgb3IgZ2V0IHlvdXIgaW50ZXJydXB0LCBvciByZWdpc3RlciBp bnRvCj4gYW55IGZyYW1ld29yayAoaW5jbHVkaW5nIGRybV9kZXZfcmVnaXN0ZXIgdGhhdCBwcmV0 dHkgbXVjaCBldmVyeSBkcml2ZXIKPiBoYW5kbGVzIHByb3Blcmx5IGlmIGl0IHJldHVybnMgYW4g ZXJyb3IsIHdpdGhvdXQgYmVpbmcgZ3JhY2VmdWwgYWJvdXQKPiBpdCkuCgpUaG9zZSBhcmUgYWxs IGR5bmFtaWMgdGhpbmdzIHRoYXQgY2FuIGZhaWwuCgpRdWl0ZSBkaWZmZXJlbnQgZnJvbSBwYXNz aW5nIE5VTEwgZGV2LCBjb25uZWN0b3IsIG9yIGZ1bmNzIHRvCmRybV9jb25uZWN0b3JfaW5pdCgp IGFuZCBmcmllbmRzLgoKSSB0aGluayBpdCdzIHdyb25nIHRvIHNldCB0aGUgZXhhbXBsZSB0aGF0 IGV2ZXJ5dGhpbmcgbmVlZHMgdG8gYmUKY2hlY2tlZCwgZXZlcnl0aGluZyBuZWVkcyB0byByZXR1 cm4gYW4gZXJyb3IsIGV2ZXJ5IGNhbGwgbmVlZHMgdG8gY2hlY2sKZm9yIGVycm9yIHJldHVybiwg YWxsIHRoZSB0aW1lLCBldmVyeXdoZXJlLiBQZW9wbGUgYWJzb2x1dGVseSB3aWxsIGNhcmdvCmN1 bHQgdGhhdCwgYW5kIHRoYXQncyB3aGF0IFZpbGxlIGlzIHJlZmVycmluZyB0by4KCklmIHlvdSBw YXNzIE5VTEwgZGV2LCBjb25uZWN0b3IsIG9yIGZ1bmNzIHRvIGRybV9jb25uZWN0b3JfaW5pdCgp IEkKdGhpbmsgeW91IGFic29sdXRlbHkgZGVzZXJ2ZSB0byBnZXQgYW4gb29wcy4KCkZvciBkZXYs IHlvdSBjb3VsZCBwb3NzaWJseSBub3QgaGF2ZSByZWFjaGVkIHRoZSBmdW5jdGlvbiB3aXRoIE5V TEwKZGV2LiAoQW5kIF9fZHJtX2Nvbm5lY3Rvcl9pbml0KCkgaGFzIGRldi0+bW9kZV9jb25maWcg YmVmb3JlIHRoZSBjaGVjaywKc28geW91J2xsIGdldCBhIHN0YXRpYyBhbmFseXplciB3YXJuaW5n IGFib3V0IGRlcmVmZXJlbmNlIGJlZm9yZSB0aGUKY2hlY2suKSBJZiB5b3UgaGF2ZSBOVUxMIGNv bm5lY3RvciwgeW91IGRpZG4ndCBjaGVjayBmb3IgYWxsb2NhdGlvbgpmYWlsdXJlIGVhcmxpZXIu IElmIHlvdSBoYXZlIE5VTEwgZnVuY3MsIHlvdSBqdXN0IHBhc3NlZCBOVUxMLCBiZWNhdXNlCml0 J3MgZ2VuZXJhbGx5IHN1cHBvc2VkIHRvIGJlIGEgcG9pbnRlciB0byBhIHN0YXRpYyBjb25zdCBz dHJ1Y3QuCgo+PiBIYXZpbmcgc3VjaCBjaGVja3MgaW4gcGxhY2UgdHJhaW5zIHBlb3BsZSB0byB0 aGluayB0aGV5ICptYXkqIGhhcHBlbi4KPgo+IEluIG1vc3QgY2FzZXMsIGttYWxsb2MgY2FuJ3Qg ZmFpbC4gV2Ugc2VlbSB0byBoYXZlIGEgdmVyeSBkaWZmZXJlbnQKPiBwb2xpY3kgdG93YXJkcyBp dC4KCkFnYWluLCBkeW5hbWljIGluIG5hdHVyZSBhbmQgY2FuIGZhaWwuCgo+PiBXaGlsZSBpdCBz aG91bGQgZmFpbCBmYXN0IGFuZCBsb3VkIGF0IHRoZSBkZXZlbG9wZXIncyBmaXJzdCBzbW9rZSB0 ZXN0LAo+PiBhbmQgZ2V0IGZpeGVkIHRoZW4gYW5kIHRoZXJlLgo+Cj4gUmV0dXJuaW5nIGFuIGVy cm9yICsgYSB3YXJuaW5nIGFsc28gcXVhbGlmaWVzIGZvciAiZmFpbCBmYXN0IGFuZCBsb3VkIi4K PiBCdXQga2VlcHMgdGhlIHN5c3RlbSBhbGl2ZSBmb3Igc29tZW9uZSB0byBub3RpY2UgaW4gYW55 IGNhc2UuCgpCdXQgd2hlcmUgZG8geW91IGRyYXcgdGhlIGxpbmU/IElmIHdlIGtlZXAgYWRkaW5n IHRoZXNlIGNoZWNrcyB0byB0aGluZ3MKdGhhdCBhY3R1YWxseSBjYW4ndCBoYXBwZW4sIHdlIHRl YWNoIGRldmVsb3BlcnMgd2UgbmVlZCB0byBjaGVjayBmb3IKaW1wb3NzaWJsZSB0aGluZ3MuIEFu ZCB3ZSB0ZWFjaCB0aGVtIG5vdCB0byB0cnVzdCBhbnl0aGluZy4KCkkgc2Nyb2xsIGRvd24gdGhl IGZpbGUgYW5kIHJlYWNoCmRybV9jb25uZWN0b3JfYXR0YWNoX2VkaWRfcHJvcGVydHkoKS4gU2hv dWxkIHdlIE5VTEwgY2hlY2sgY29ubmVjdG9yPwpTaG91bGQgd2UgY2hhbmdlIHRoZSBmdW5jdGlv biB0byBpbnQgYW5kIHJldHVybiBhIHZhbHVlPyBTaG91bGQgdGhlCmNhbGxlciBjaGVjayB0aGUg dmFsdWU/IFRoZW4gdGhlcmUncyBkcm1fY29ubmVjdG9yX2F0dGFjaF9lbmNvZGVyKCkuIEFuZApk cm1fY29ubmVjdG9yX2hhc19wb3NzaWJsZV9lbmNvZGVyKCkuIEFuZCBzbyBvbiBhbmQgc28gZm9y dGguCgpXaGVyZSBkbyB5b3UgZHJhdyB0aGUgbGluZT8KCgpCUiwKSmFuaS4KCgotLSAKSmFuaSBO aWt1bGEsIEludGVsCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpMaW51eC1yb2NrY2hpcCBtYWlsaW5nIGxpc3QKTGludXgtcm9ja2NoaXBAbGlzdHMuaW5m cmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xp bnV4LXJvY2tjaGlwCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 15910C07CB1 for ; Wed, 29 Nov 2023 11:41:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ocrmkmQR0xv/qPlIHNLHCxdUZ1jRYi8PN8MsZGeGr58=; b=MXolu7g87eyLVw 5h4jYn+TyZbUEZiybLtm4X19+XXWwKeEZomF9CJz5yq2+YqGhRvw2MviB9bZ1JX9iCdOLgeYQnG24 XtD2uKLwKWE1x1m1QM2op3ocUKpRihHmTCimXMjDcbO1Sokuxpq34FunIBmpg6ezYUlj6j00JUwYl +7DxxUGR0+ydE+CVjlzjBQx2JRT4+bPmZWRj2yjIaLQ3VPgJdSUw4ug3Vk5qDp1+Kj3zAUHqiDmKS iu5m0KBv5kIzkKfGicFcK0ChPbWnuJ00pJHrXc+AwnwkxD1BfdDmz3DtX1v4IhsGalRH5BLIFVJ6O j5qkDe/oLiAylARQJM1w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8Iw0-0089qS-26; Wed, 29 Nov 2023 11:40:52 +0000 Received: from mgamail.intel.com ([198.175.65.11]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r8Ivw-0089pm-2s; Wed, 29 Nov 2023 11:40:50 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701258049; x=1732794049; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=w91TqZU/Sz95TBAtE+lLgPRTyMnYI3/7ak+ZXvIxwqY=; b=CeWDV1pgm/8AvvRPX7R6ba9TLeyFX/w6u3MSVW0tp/iZRwXRKt5xvLMt NhHCNQ3F38HRkWOYMVsA71Iz736zA4cPMIdX0W7fxzLWhmbX8por6aNF8 BP+pSevsMMNHsPim+Og1qVWNqHvTR7sz2wfOZeeFtqL+qHg4rs/uMhphC evTSFy5X2iBodMO0u5XppzfoFLM0CBQeN/mMsaUcgYZvfk9RlGh8+IBu0 HeRbtSYdeCr1tgtMvpqxSX8pGutQip75GQ6Td0kK4nXmmGOKou4Xwpg7o Gl5y7qpJga0zgmJUh5XL1EAdXFNjJygjNZntVyHreXwnLlKrVNuSJVNzi Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="35043" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="35043" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="942291528" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="942291528" Received: from dstavrak-mobl.ger.corp.intel.com (HELO localhost) ([10.252.60.61]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:40 -0800 From: Jani Nikula To: Maxime Ripard Cc: Ville =?utf-8?B?U3lyasOkbMOk?= , Thomas Zimmermann , Emma Anholt , Jonathan Corbet , linux-kernel@vger.kernel.org, Samuel Holland , Sandy Huang , Jernej Skrabec , linux-doc@vger.kernel.org, Hans Verkuil , linux-rockchip@lists.infradead.org, Chen-Yu Tsai , dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 05/45] drm/connector: Check drm_connector_init pointers arguments In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20231128-kms-hdmi-connector-state-v4-0-c7602158306e@kernel.org> <20231128-kms-hdmi-connector-state-v4-5-c7602158306e@kernel.org> <87h6l66nth.fsf@intel.com> <2mnodqvu2oo674vspiy4gxhglu3it5cq47acx5itnbwevgc4cf@c7h2bvnx3m2n> <8734wo7vbx.fsf@intel.com> Date: Wed, 29 Nov 2023 13:40:38 +0200 Message-ID: <87ttp46b49.fsf@intel.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231129_034049_002406_02BCDFF0 X-CRM114-Status: GOOD ( 41.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org T24gV2VkLCAyOSBOb3YgMjAyMywgTWF4aW1lIFJpcGFyZCA8bXJpcGFyZEBrZXJuZWwub3JnPiB3 cm90ZToKPiBPbiBXZWQsIE5vdiAyOSwgMjAyMyBhdCAxMTozODo0MkFNICswMjAwLCBKYW5pIE5p a3VsYSB3cm90ZToKPj4gT24gV2VkLCAyOSBOb3YgMjAyMywgTWF4aW1lIFJpcGFyZCA8bXJpcGFy ZEBrZXJuZWwub3JnPiB3cm90ZToKPj4gPiBIaSBWaWxsZSwKPj4gPgo+PiA+IE9uIFR1ZSwgTm92 IDI4LCAyMDIzIGF0IDAzOjQ5OjA4UE0gKzAyMDAsIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPj4g Pj4gT24gVHVlLCBOb3YgMjgsIDIwMjMgYXQgMDI6Mjk6NDBQTSArMDEwMCwgTWF4aW1lIFJpcGFy ZCB3cm90ZToKPj4gPj4gPiBPbiBUdWUsIE5vdiAyOCwgMjAyMyBhdCAwMjo1NDowMlBNICswMjAw LCBKYW5pIE5pa3VsYSB3cm90ZToKPj4gPj4gPiA+IE9uIFR1ZSwgMjggTm92IDIwMjMsIE1heGlt ZSBSaXBhcmQgPG1yaXBhcmRAa2VybmVsLm9yZz4gd3JvdGU6Cj4+ID4+ID4gPiA+IEFsbCB0aGUg ZHJtX2Nvbm5lY3Rvcl9pbml0IHZhcmlhbnRzIHRha2UgYXQgbGVhc3QgYSBwb2ludGVyIHRvIHRo ZQo+PiA+PiA+ID4gPiBkZXZpY2UsIGNvbm5lY3RvciBhbmQgaG9va3MgaW1wbGVtZW50YXRpb24u Cj4+ID4+ID4gPiA+Cj4+ID4+ID4gPiA+IEhvd2V2ZXIsIG5vbmUgb2YgdGhlbSBjaGVjayB0aGVp ciB2YWx1ZSBiZWZvcmUgZGVyZWZlcmVuY2luZyB0aG9zZQo+PiA+PiA+ID4gPiBwb2ludGVycyB3 aGljaCBjYW4gbGVhZCB0byBhIE5VTEwtcG9pbnRlciBkZXJlZmVyZW5jZSBpZiB0aGUgYXV0aG9y Cj4+ID4+ID4gPiA+IGlzbid0IGNhcmVmdWwuCj4+ID4+ID4gPiAKPj4gPj4gPiA+IEFyZ3VhYmx5 IG9vcHNpbmcgb24gdGhlIHNwb3QgaXMgcHJlZmVycmFibGUgd2hlbiB0aGlzIGNhbid0IGJlIGNh dXNlZCBieQo+PiA+PiA+ID4gdXNlciBpbnB1dC4gSXQncyBhbHdheXMgYSBtaXN0YWtlIHRoYXQg c2hvdWxkIGJlIGNhdWdodCBlYXJseSBkdXJpbmcKPj4gPj4gPiA+IGRldmVsb3BtZW50Lgo+PiA+ PiA+ID4gCj4+ID4+ID4gPiBOb3QgZXZlcnlvbmUgY2hlY2tzIHRoZSByZXR1cm4gdmFsdWUgb2Yg ZHJtX2Nvbm5lY3Rvcl9pbml0IGFuZCBmcmllbmRzLAo+PiA+PiA+ID4gc28gdGhvc2UgY2FzZXMg d2lsbCBsZWFkIHRvIG1vcmUgbXlzdGVyaW91cyBidWdzIGxhdGVyLiBBbmQgcHJvYmFibHkKPj4g Pj4gPiA+IG9vcHNlcyBhcyB3ZWxsLgo+PiA+PiA+IAo+PiA+PiA+IFNvIG1heWJlIHdlIGNhbiBk byBib3RoIHRoZW4sIHdpdGggc29tZXRoaW5nIGxpa2UKPj4gPj4gPiAKPj4gPj4gPiBpZiAoV0FS Tl9PTighZGV2KSkKPj4gPj4gPiAgICByZXR1cm4gLUVJTlZBTAo+PiA+PiA+IAo+PiA+PiA+IGlm IChkcm1fV0FSTl9PTihkZXYsICFjb25uZWN0b3IgfHwgIWZ1bmNzKSkKPj4gPj4gPiAgICByZXR1 cm4gLUVJTlZBTDsKPj4gPj4gPiAKPj4gPj4gPiBJJ2Qgc3RpbGwgbGlrZSB0byBjaGVjayBmb3Ig dGhpcywgc28gd2UgY2FuIGhhdmUgcHJvcGVyIHRlc3RpbmcsIGFuZCB3ZQo+PiA+PiA+IGFscmVh ZHkgY2hlY2sgZm9yIHRob3NlIHBvaW50ZXJzIGluIHNvbWUgcGxhY2VzIChsaWtlIGZ1bmNzIGlu Cj4+ID4+ID4gZHJtX2Nvbm5lY3Rvcl9pbml0KSwgc28gaWYgd2UgZG9uJ3QgY292ZXIgZXZlcnl0 aGluZyB3ZSdyZSBpbmNvbnNpc3RlbnQuCj4+ID4+IAo+PiA+PiBQZW9wbGUgd2lsbCBpbnZhcmlh Ymx5IGNhcmdvLWN1bHQgdGhpcyBraW5kIG9mIHN0dWZmIGFic29sdXRlbHkKPj4gPj4gZXZlcnl3 aGVyZSBhbmQgdGhlbiBhbGwgeW91ciBmdW5jdGlvbnMgd2lsbCBoYXZlIHRvbnMgb2YgZGVhZAo+ PiA+PiBjb2RlIHRvIGNoZWNrIHRoZWlyIGFyZ3VtZW50cy4KPj4gPgo+PiA+IEFuZCB0aGF0J3Mg YSBiYWQgdGhpbmcgYmVjYXVzZS4uLiA/Cj4+ID4KPj4gPiBBbHNvLCBhcmUgeW91IHJlYWxseSBz YXlpbmcgdGhhdCBjaGVja2luZyB0aGF0IHlvdXIgYXJndW1lbnRzIG1ha2Ugc2Vuc2UKPj4gPiBp cyBjYXJnby1jdWx0Pwo+PiAKPj4gSXQncyBhIHBvd2VyZnVsIHRoaW5nIHRvIGJlIGFibGUgdG8g YXNzdW1lIGEgTlVMTCBhcmd1bWVudCBpcyBhbHdheXMgYQo+PiBmYXRhbCBwcm9ncmFtbWluZyBl cnJvciBvbiB0aGUgY2FsbGVyJ3Mgc2lkZSwgYW5kIHNob3VsZCBvb3BzIGFuZCBnZXQKPj4gY2F1 Z2h0IGltbWVkaWF0ZWx5LiBJdCdzIGFuIGFzc2VydGlvbi4KPgo+IFllYWgsIGJ1dCB3ZSdyZSBu b3QgcmVhbGx5IGRvaW5nIHRoYXQgZWl0aGVyLiBXZSBoYXZlIG5vIGV4cGxpY2l0Cj4gYXNzZXJ0 aW9uIGFueXdoZXJlLiBXZSB0YWtlIGEgcG9pbnRlciBpbiwgYW5kIGp1c3QgaG9wZSB0aGF0IGl0 IHdpbGwgYmUKPiBkZXJlZmVyZW5jZWQgbGF0ZXIgb24gYW5kIHRoYXQgdGhlIGtlcm5lbCB3aWxs IGNyYXNoLiBUaGUgcG9pbnRlciB0byB0aGUKPiBmdW5jdGlvbnMgZXNwZWNpYWxseSBpcyBvbmx5 IGRlZmVyZW5jZWQgdmVyeSBsYXRlciBvbi4KPgo+IEFuZCBhc3NlcnRpb25zIG1pZ2h0IGJlIHBv d2VyZnVsLCBidXQgYmVpbmcgYWJsZSB0byBub3RpY2UgZXJyb3JzIGFuZAo+IGRlYnVnIHRoZW0g aXMgdG9vLiBBIHBhbmljIHRha2VzIGF3YXkgYmFzaWNhbGx5IGFueSByZW1vdGUgYWNjZXNzIHRv Cj4gZGVidWcuIElmIHlvdSBkb24ndCBoYXZlIGEgY29uc29sZSwgeW91J3JlIGRvbmUuCj4KPj4g V2UncmUgbm90IHRhbGtpbmcgYWJvdXQgdXNlciBpbnB1dCBvciBhbnl0aGluZyBsaWtlIHRoYXQg aGVyZS4KPj4gCj4+IElmIHlvdSBzdGFydCBjaGVja2luZyBmb3IgdGhpbmdzIHRoYXQgY2FuJ3Qg aGFwcGVuLCBhbmQgcmV0dXJuIGVycm9ycwo+PiBmb3IgdGhlbSwgeW91IHN0YXJ0IGdyYWNlZnVs bHkgaGFuZGxpbmcgdGhpbmdzIHRoYXQgZG9uJ3QgaGF2ZSBhbnl0aGluZwo+PiBncmFjZWZ1bCBh Ym91dCB0aGVtLgo+Cj4gQnV0IHRoZXJlJ3Mgbm90aGluZyBncmFjZWZ1bCB0byBkbyBoZXJlOiB5 b3UganVzdCByZXR1cm4gZnJvbSB5b3VyIHByb2JlCj4gZnVuY3Rpb24gdGhhdCB5b3UgY291bGRu J3QgcHJvYmUgYW5kIHRoYXQncyBpdC4gSnVzdCBsaWtlIHlvdSBkbyB3aGVuCj4geW91IGNhbid0 IG1hcCB5b3VyIHJlZ2lzdGVycywgb3IgZ2V0IHlvdXIgaW50ZXJydXB0LCBvciByZWdpc3RlciBp bnRvCj4gYW55IGZyYW1ld29yayAoaW5jbHVkaW5nIGRybV9kZXZfcmVnaXN0ZXIgdGhhdCBwcmV0 dHkgbXVjaCBldmVyeSBkcml2ZXIKPiBoYW5kbGVzIHByb3Blcmx5IGlmIGl0IHJldHVybnMgYW4g ZXJyb3IsIHdpdGhvdXQgYmVpbmcgZ3JhY2VmdWwgYWJvdXQKPiBpdCkuCgpUaG9zZSBhcmUgYWxs IGR5bmFtaWMgdGhpbmdzIHRoYXQgY2FuIGZhaWwuCgpRdWl0ZSBkaWZmZXJlbnQgZnJvbSBwYXNz aW5nIE5VTEwgZGV2LCBjb25uZWN0b3IsIG9yIGZ1bmNzIHRvCmRybV9jb25uZWN0b3JfaW5pdCgp IGFuZCBmcmllbmRzLgoKSSB0aGluayBpdCdzIHdyb25nIHRvIHNldCB0aGUgZXhhbXBsZSB0aGF0 IGV2ZXJ5dGhpbmcgbmVlZHMgdG8gYmUKY2hlY2tlZCwgZXZlcnl0aGluZyBuZWVkcyB0byByZXR1 cm4gYW4gZXJyb3IsIGV2ZXJ5IGNhbGwgbmVlZHMgdG8gY2hlY2sKZm9yIGVycm9yIHJldHVybiwg YWxsIHRoZSB0aW1lLCBldmVyeXdoZXJlLiBQZW9wbGUgYWJzb2x1dGVseSB3aWxsIGNhcmdvCmN1 bHQgdGhhdCwgYW5kIHRoYXQncyB3aGF0IFZpbGxlIGlzIHJlZmVycmluZyB0by4KCklmIHlvdSBw YXNzIE5VTEwgZGV2LCBjb25uZWN0b3IsIG9yIGZ1bmNzIHRvIGRybV9jb25uZWN0b3JfaW5pdCgp IEkKdGhpbmsgeW91IGFic29sdXRlbHkgZGVzZXJ2ZSB0byBnZXQgYW4gb29wcy4KCkZvciBkZXYs IHlvdSBjb3VsZCBwb3NzaWJseSBub3QgaGF2ZSByZWFjaGVkIHRoZSBmdW5jdGlvbiB3aXRoIE5V TEwKZGV2LiAoQW5kIF9fZHJtX2Nvbm5lY3Rvcl9pbml0KCkgaGFzIGRldi0+bW9kZV9jb25maWcg YmVmb3JlIHRoZSBjaGVjaywKc28geW91J2xsIGdldCBhIHN0YXRpYyBhbmFseXplciB3YXJuaW5n IGFib3V0IGRlcmVmZXJlbmNlIGJlZm9yZSB0aGUKY2hlY2suKSBJZiB5b3UgaGF2ZSBOVUxMIGNv bm5lY3RvciwgeW91IGRpZG4ndCBjaGVjayBmb3IgYWxsb2NhdGlvbgpmYWlsdXJlIGVhcmxpZXIu IElmIHlvdSBoYXZlIE5VTEwgZnVuY3MsIHlvdSBqdXN0IHBhc3NlZCBOVUxMLCBiZWNhdXNlCml0 J3MgZ2VuZXJhbGx5IHN1cHBvc2VkIHRvIGJlIGEgcG9pbnRlciB0byBhIHN0YXRpYyBjb25zdCBz dHJ1Y3QuCgo+PiBIYXZpbmcgc3VjaCBjaGVja3MgaW4gcGxhY2UgdHJhaW5zIHBlb3BsZSB0byB0 aGluayB0aGV5ICptYXkqIGhhcHBlbi4KPgo+IEluIG1vc3QgY2FzZXMsIGttYWxsb2MgY2FuJ3Qg ZmFpbC4gV2Ugc2VlbSB0byBoYXZlIGEgdmVyeSBkaWZmZXJlbnQKPiBwb2xpY3kgdG93YXJkcyBp dC4KCkFnYWluLCBkeW5hbWljIGluIG5hdHVyZSBhbmQgY2FuIGZhaWwuCgo+PiBXaGlsZSBpdCBz aG91bGQgZmFpbCBmYXN0IGFuZCBsb3VkIGF0IHRoZSBkZXZlbG9wZXIncyBmaXJzdCBzbW9rZSB0 ZXN0LAo+PiBhbmQgZ2V0IGZpeGVkIHRoZW4gYW5kIHRoZXJlLgo+Cj4gUmV0dXJuaW5nIGFuIGVy cm9yICsgYSB3YXJuaW5nIGFsc28gcXVhbGlmaWVzIGZvciAiZmFpbCBmYXN0IGFuZCBsb3VkIi4K PiBCdXQga2VlcHMgdGhlIHN5c3RlbSBhbGl2ZSBmb3Igc29tZW9uZSB0byBub3RpY2UgaW4gYW55 IGNhc2UuCgpCdXQgd2hlcmUgZG8geW91IGRyYXcgdGhlIGxpbmU/IElmIHdlIGtlZXAgYWRkaW5n IHRoZXNlIGNoZWNrcyB0byB0aGluZ3MKdGhhdCBhY3R1YWxseSBjYW4ndCBoYXBwZW4sIHdlIHRl YWNoIGRldmVsb3BlcnMgd2UgbmVlZCB0byBjaGVjayBmb3IKaW1wb3NzaWJsZSB0aGluZ3MuIEFu ZCB3ZSB0ZWFjaCB0aGVtIG5vdCB0byB0cnVzdCBhbnl0aGluZy4KCkkgc2Nyb2xsIGRvd24gdGhl IGZpbGUgYW5kIHJlYWNoCmRybV9jb25uZWN0b3JfYXR0YWNoX2VkaWRfcHJvcGVydHkoKS4gU2hv dWxkIHdlIE5VTEwgY2hlY2sgY29ubmVjdG9yPwpTaG91bGQgd2UgY2hhbmdlIHRoZSBmdW5jdGlv biB0byBpbnQgYW5kIHJldHVybiBhIHZhbHVlPyBTaG91bGQgdGhlCmNhbGxlciBjaGVjayB0aGUg dmFsdWU/IFRoZW4gdGhlcmUncyBkcm1fY29ubmVjdG9yX2F0dGFjaF9lbmNvZGVyKCkuIEFuZApk cm1fY29ubmVjdG9yX2hhc19wb3NzaWJsZV9lbmNvZGVyKCkuIEFuZCBzbyBvbiBhbmQgc28gZm9y dGguCgpXaGVyZSBkbyB5b3UgZHJhdyB0aGUgbGluZT8KCgpCUiwKSmFuaS4KCgotLSAKSmFuaSBO aWt1bGEsIEludGVsCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3Rz LmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5m by9saW51eC1hcm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8DC9AC4167B for ; Wed, 29 Nov 2023 11:47:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AF7B110E1BF; Wed, 29 Nov 2023 11:47:55 +0000 (UTC) X-Greylist: delayed 425 seconds by postgrey-1.36 at gabe; Wed, 29 Nov 2023 11:47:53 UTC Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) by gabe.freedesktop.org (Postfix) with ESMTPS id 247B210E1B1 for ; Wed, 29 Nov 2023 11:47:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701258473; x=1732794473; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=w91TqZU/Sz95TBAtE+lLgPRTyMnYI3/7ak+ZXvIxwqY=; b=bo7f2LEgV0mBD3AP7x4Ihoo28NMmydC0Z+FywPyTRoYVRUc09tVTX+xA Tn8nDrqrek12Ak8Du+sKA8YTZurQL+MkG7o16B1U7eSnGxs4khCWDnOVG D7a3JJy15+plo18VxPxH6V1ewi0lTCeeVnJS7At3aBr71RAwoX1EFH/eL vav7j/Cl+Wj0LwSuFM/KhnqrTmscz9+rH7gqOir2BJTuE84TvVO21+o9F uAPdn8g4naaEk7QJqUnQFo4Pv3gaswcgNx0IvERuAAfxhp5pRYQfi8HTj 89rvss1l54TzcU9FnyxcIraawRrQXZK5YsHyf5cLNjQEWg95eUBPUchH2 w==; X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="35041" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="35041" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10908"; a="942291528" X-IronPort-AV: E=Sophos;i="6.04,235,1695711600"; d="scan'208";a="942291528" Received: from dstavrak-mobl.ger.corp.intel.com (HELO localhost) ([10.252.60.61]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Nov 2023 03:40:40 -0800 From: Jani Nikula To: Maxime Ripard Subject: Re: [PATCH v4 05/45] drm/connector: Check drm_connector_init pointers arguments In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20231128-kms-hdmi-connector-state-v4-0-c7602158306e@kernel.org> <20231128-kms-hdmi-connector-state-v4-5-c7602158306e@kernel.org> <87h6l66nth.fsf@intel.com> <2mnodqvu2oo674vspiy4gxhglu3it5cq47acx5itnbwevgc4cf@c7h2bvnx3m2n> <8734wo7vbx.fsf@intel.com> Date: Wed, 29 Nov 2023 13:40:38 +0200 Message-ID: <87ttp46b49.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arm-kernel@lists.infradead.org, Emma Anholt , Samuel Holland , Jonathan Corbet , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, Jernej Skrabec , Sandy Huang , Hans Verkuil , linux-rockchip@lists.infradead.org, Chen-Yu Tsai , dri-devel@lists.freedesktop.org, Thomas Zimmermann , linux-sunxi@lists.linux.dev, linux-media@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, 29 Nov 2023, Maxime Ripard wrote: > On Wed, Nov 29, 2023 at 11:38:42AM +0200, Jani Nikula wrote: >> On Wed, 29 Nov 2023, Maxime Ripard wrote: >> > Hi Ville, >> > >> > On Tue, Nov 28, 2023 at 03:49:08PM +0200, Ville Syrj=C3=A4l=C3=A4 wrot= e: >> >> On Tue, Nov 28, 2023 at 02:29:40PM +0100, Maxime Ripard wrote: >> >> > On Tue, Nov 28, 2023 at 02:54:02PM +0200, Jani Nikula wrote: >> >> > > On Tue, 28 Nov 2023, Maxime Ripard wrote: >> >> > > > All the drm_connector_init variants take at least a pointer to = the >> >> > > > device, connector and hooks implementation. >> >> > > > >> >> > > > However, none of them check their value before dereferencing th= ose >> >> > > > pointers which can lead to a NULL-pointer dereference if the au= thor >> >> > > > isn't careful. >> >> > >=20 >> >> > > Arguably oopsing on the spot is preferrable when this can't be ca= used by >> >> > > user input. It's always a mistake that should be caught early dur= ing >> >> > > development. >> >> > >=20 >> >> > > Not everyone checks the return value of drm_connector_init and fr= iends, >> >> > > so those cases will lead to more mysterious bugs later. And proba= bly >> >> > > oopses as well. >> >> >=20 >> >> > So maybe we can do both then, with something like >> >> >=20 >> >> > if (WARN_ON(!dev)) >> >> > return -EINVAL >> >> >=20 >> >> > if (drm_WARN_ON(dev, !connector || !funcs)) >> >> > return -EINVAL; >> >> >=20 >> >> > I'd still like to check for this, so we can have proper testing, an= d we >> >> > already check for those pointers in some places (like funcs in >> >> > drm_connector_init), so if we don't cover everything we're inconsis= tent. >> >>=20 >> >> People will invariably cargo-cult this kind of stuff absolutely >> >> everywhere and then all your functions will have tons of dead >> >> code to check their arguments. >> > >> > And that's a bad thing because... ? >> > >> > Also, are you really saying that checking that your arguments make sen= se >> > is cargo-cult? >>=20 >> It's a powerful thing to be able to assume a NULL argument is always a >> fatal programming error on the caller's side, and should oops and get >> caught immediately. It's an assertion. > > Yeah, but we're not really doing that either. We have no explicit > assertion anywhere. We take a pointer in, and just hope that it will be > dereferenced later on and that the kernel will crash. The pointer to the > functions especially is only deferenced very later on. > > And assertions might be powerful, but being able to notice errors and > debug them is too. A panic takes away basically any remote access to > debug. If you don't have a console, you're done. > >> We're not talking about user input or anything like that here. >>=20 >> If you start checking for things that can't happen, and return errors >> for them, you start gracefully handling things that don't have anything >> graceful about them. > > But there's nothing graceful to do here: you just return from your probe > function that you couldn't probe and that's it. Just like you do when > you can't map your registers, or get your interrupt, or register into > any framework (including drm_dev_register that pretty much every driver > handles properly if it returns an error, without being graceful about > it). Those are all dynamic things that can fail. Quite different from passing NULL dev, connector, or funcs to drm_connector_init() and friends. I think it's wrong to set the example that everything needs to be checked, everything needs to return an error, every call needs to check for error return, all the time, everywhere. People absolutely will cargo cult that, and that's what Ville is referring to. If you pass NULL dev, connector, or funcs to drm_connector_init() I think you absolutely deserve to get an oops. For dev, you could possibly not have reached the function with NULL dev. (And __drm_connector_init() has dev->mode_config before the check, so you'll get a static analyzer warning about dereference before the check.) If you have NULL connector, you didn't check for allocation failure earlier. If you have NULL funcs, you just passed NULL, because it's generally supposed to be a pointer to a static const struct. >> Having such checks in place trains people to think they *may* happen. > > In most cases, kmalloc can't fail. We seem to have a very different > policy towards it. Again, dynamic in nature and can fail. >> While it should fail fast and loud at the developer's first smoke test, >> and get fixed then and there. > > Returning an error + a warning also qualifies for "fail fast and loud". > But keeps the system alive for someone to notice in any case. But where do you draw the line? If we keep adding these checks to things that actually can't happen, we teach developers we need to check for impossible things. And we teach them not to trust anything. I scroll down the file and reach drm_connector_attach_edid_property(). Should we NULL check connector? Should we change the function to int and return a value? Should the caller check the value? Then there's drm_connector_attach_encoder(). And drm_connector_has_possible_encoder(). And so on and so forth. Where do you draw the line? BR, Jani. --=20 Jani Nikula, Intel