From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 62AB93F44F6 for ; Thu, 11 Jun 2026 12:45:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781181943; cv=none; b=Uv0L/quYAAi2wLY3zHafiKIcyayM5R4yeMJP2es/iB6YhP2601AJ3WbYrWrSEBSTShhN+Hz68xFsJZjmlLIc/2S1ipagKRWmWWIGOK+BR+02IeX0wKV34Alcfz55HJ+LTTo88stA8DcVyQrRmftwbWzsa8vMf6WTj9airYpyQeQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781181943; c=relaxed/simple; bh=0QpWHanUctIDYODCq75PFb6Mpggc+wnyVuQmY+hgE0g=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=lYt5i6OUTTeZ2rzxONW7M4W9W/Cxih9lnNxif6OgSCqa/N89CO27XN0IDxLAx2zoZy5xPlL7u0iaIcUM9NBuozXkx8J6SAdbw6xV6z7o7EQ6WopQu6x307roBjXlEHaTkv4DgmYnmtBqklOtMQ7n3mNsM1xvjUy1I4tLLgQx8co= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Xsu5KuNF; arc=none smtp.client-ip=209.85.221.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Xsu5KuNF" Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-45ef0ccccfcso560362f8f.0 for ; Thu, 11 Jun 2026 05:45:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1781181940; x=1781786740; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=cZC8AOgwDUkqE3FrJgyPhYy8Ulxsg9q9DK142dUYM4M=; b=Xsu5KuNFkzDhoaqLZM3b/u0M+gXx30a8tpjk6N7ENyH0FuyZ7RqwvYtt3WGOV1NudF iLSdGdVCdauBND69XzcsR5P8/emWfAVifilRqm+SBNxOUS6L7vWl4gzdJNEY7ZRn6oAl 5Jv1i/WcWq2lX7xNoQruFOOId/JmjJ3VlqpLD1eY/XhiCOo+SWbg3OXWR7I2BUaK1FWR yW2eLRsu9NnUNrAJ29+7BP64oLab0K/+scAEdi9XD7c1B3zArQGTQfd2EJHLFtaDh8WY 9cNswb3FOrUsIiD1VVRCQffGBmEeNrrJxFdtHeHNrIeSSlnrQJDZ17zomeCfXayBp4tu dmtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781181940; x=1781786740; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=cZC8AOgwDUkqE3FrJgyPhYy8Ulxsg9q9DK142dUYM4M=; b=otm46cU9TK3rtDyppLUI94lENRdMtRpFR1SHxTxiudu0Ryqp2BTdMdJ3dQ7Yu7EOOC HUc0JFZRZELolmq74NRQpvhpTU5o9dEzYG8yn4XAlBhTeifjihrlaAcpv9w1fdzj8ipi 6hldWaIaofV2bzv9B5IpXgRCt0x5OK6eJDoQbk+fYvQ/aMzLk6HpG+g4N9aykwZU2+Uq wOdp7OhHqxBFk01HuCObaK1V7OcP1a7iCPmc7XGg7ZytrwIW5ql7Yu/cT8BtbzVWWGHj MOjRy63wAn5Sb97Ego13fzOfNnO/MunEQz0nhXPphvWSbGCyJz2SolLGz0tOYzBSMJyO RIrw== X-Forwarded-Encrypted: i=1; AFNElJ8jQ5SMbDLrOacCHAt/vHwx6EIr4ZpCl1M6D9I6MXbcG4HhtlQYUTjCO34R/MjICQuZSX+1I6tt2XV+b4g=@vger.kernel.org X-Gm-Message-State: AOJu0YxSzvoMNlsb78TpY3saQLPiJvnt4smHL4sbKvbCSs3WFdIuJyBb Nb8qmtJUJZpPa/cO2bWyC2sXF8n/tjesALqGUJSS49PNrO7U6uraf9Tzhwngqy4o61A= X-Gm-Gg: Acq92OHSH8GYqZtxdY/n/ZkckKxd5zDOgSwVjazdPLQqUuxwi36nxHRIHsVXRXZTUmB lwJWn5T3hFxkEb0ikqpqml6CqvTm79IlibNVilKs1Nxg/b/e6VI6yN5TLJ/hx/d3/amvGew0yqr 5VY4g9IpsJZbjJGUCnknjKKr6KFbng48pKElU7OzY4Gb7VG4ah1zFK3vGrbdCaLLqjixNNA4IgT uvi8ZREkMAla6pOdGrVYhuXXJ6aZhFol9wyPM6zio6EU8dgti6/6RuBGXQvLCCh9C6J5nDtrYWE c1tB/000Zz5oVU3/3hljcMhHS3lU3BR7FKlD8HCbiJRHEkeKag7ZSKROaDHOX/bGziPfMiaLPpf wx7Usi2fP6VrbWCgZyajLTqt6kedHBUNd/cTymmu8OnmZONHD+TYcxY58DGGVod5uqmDK4D8teU 8//ssAm2vUdKzWbBz5aXkyX20a2SHtTKvkBXt8ZJpFcxGyO5s8tWNl5aYiILzDyW1zZmaOpaimO Wf4qonbua0eE984vgrWTGSxCThh X-Received: by 2002:a05:600c:1d16:b0:490:e919:7bba with SMTP id 5b1f17b1804b1-490e9197c83mr3700335e9.8.1781181939386; Thu, 11 Jun 2026 05:45:39 -0700 (PDT) Received: from localhost.localdomain (ip-109-193-028-127.um39.pools.vodafone-ip.de. [109.193.28.127]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-490e532c778sm48697645e9.14.2026.06.11.05.45.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jun 2026 05:45:38 -0700 (PDT) From: Marek Czernohous To: Lyude Paul , Danilo Krummrich Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org, linux-kernel@vger.kernel.org, Fab Stz Subject: [PATCH v3 2/2] drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable() Date: Thu, 11 Jun 2026 14:45:35 +0200 Message-ID: <20260611124535.527275-3-mczernohous@gmail.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260611124535.527275-1-mczernohous@gmail.com> References: <20260611124535.527275-1-mczernohous@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: Marek Czernohous nv50_sor_atomic_disable() unconditionally computes nv50_head(nv_encoder->crtc) and dereferences the result a few lines later. nv_encoder->crtc is nouveau's own shadow pointer, set in .atomic_enable and cleared at the end of .atomic_disable. Commit f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks") removed the NULL check here, reasoning that the atomic helpers never call ->disable without a crtc. On NVAC (MCP79) under Wayland sessions (observed with Weston's DRM backend and with labwc/wlroots) we have hit the NULL case in practice during session teardown and VT switches: disable runs without (or after) its matching enable, and because nv50_head() is container_of(), the NULL does not stay NULL but becomes a bogus non-NULL pointer, so the subsequent head dereferences fault and the kernel oopses. Restore the guard, as drm_WARN_ON_ONCE() instead of a silent return: a NULL crtc here still indicates a state-tracking inconsistency that should stay visible. Return without touching the output; in this path either enable never ran (nothing to tear down) or an earlier disable already did the teardown, and the encoder release is handled by the commit_tail release loop in both cases. (That loop then rejects the release of a never-acquired output with -EINVAL in the nvif layer, which is harmless; the vanilla code oopsed before ever reaching it.) The same inconsistent-state path can also leave the encoder without an old connector state, in which case nv50_outp_get_old_connector() returns NULL while the backlight teardown dereferenced it unconditionally, so the oops would only have moved there. Hoist the guard above all of that and look at the old connector only after checking it. Fixes: f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks") Cc: Tested-by: Fab Stz Assisted-by: Claude:claude-opus-4-7 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Marek Czernohous --- v2 -> v3: hoisted the guard above the CONFIG_DRM_NOUVEAU_BACKLIGHT block and made the old-connector lookup NULL-safe (reported by the Sashiko AI review on the v2 posting: the unconditional nv_connector->backlight initializer ran before the guard); folded review nits (pointer wording, release-loop note, comment). Tested-by carried over; the changes stay confined to the inconsistent-state path, normal-path behavior is unchanged. v1 -> v2: dropped the nvif_outp_release() call from the early return (the release is owned by the commit_tail release loop; calling it here released twice and detached the OR before the disable flush); turned the silent return into drm_WARN_ON_ONCE(); reworded the commit message. v2: https://lore.kernel.org/all/66ea307dd4fa080db27b2b9a0caa31b562d72c2b.1781162589.git.marek@czernohous.de/ v1: https://lore.kernel.org/nouveau/20260409172126.115441-3-marek@czernohous.de/ drivers/gpu/drm/nouveau/dispnv50/disp.c | 30 ++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 6c3a871..47e15a4 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1565,16 +1565,36 @@ static void nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder); - struct nv50_head *head = nv50_head(nv_encoder->crtc); + struct nv50_head *head; #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT - struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder); + struct nouveau_connector *nv_connector; struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev); - struct nouveau_backlight *backlight = nv_connector->backlight; - struct drm_dp_aux *aux = &nv_connector->aux; + struct nouveau_backlight *backlight; int ret; +#endif + /* nv_encoder->crtc is the driver's shadow pointer, set in + * .atomic_enable (and by the boot-time hardware readback) and + * cleared at the end of this function. NULL here + * means disable-without-enable or a double disable; bail before + * container_of() turns it into a bogus head pointer (checking the + * result would not work, container_of(NULL) is never NULL). The + * encoder release is handled by the commit_tail release loop, so + * there is nothing to clean up here. + */ + if (drm_WARN_ON_ONCE(encoder->dev, !nv_encoder->crtc)) + return; + head = nv50_head(nv_encoder->crtc); + +#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT + /* The same inconsistent-state path can leave us without an old + * connector state, so check before touching it. + */ + nv_connector = nv50_outp_get_old_connector(state, nv_encoder); + backlight = nv_connector ? nv_connector->backlight : NULL; if (backlight && backlight->uses_dpcd) { - ret = drm_edp_backlight_disable(aux, &backlight->edp_info); + ret = drm_edp_backlight_disable(&nv_connector->aux, + &backlight->edp_info); if (ret < 0) NV_ERROR(drm, "Failed to disable backlight on [CONNECTOR:%d:%s]: %d\n", nv_connector->base.base.id, nv_connector->base.name, ret); -- 2.53.0