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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C42EDC001DF for ; Tue, 25 Jul 2023 11:47:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232992AbjGYLrc (ORCPT ); Tue, 25 Jul 2023 07:47:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55332 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232742AbjGYLrU (ORCPT ); Tue, 25 Jul 2023 07:47:20 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B373411F for ; Tue, 25 Jul 2023 04:47:15 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 289AF6167D for ; Tue, 25 Jul 2023 11:47:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3B7F5C433C8; Tue, 25 Jul 2023 11:47:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1690285634; bh=gbFXmIFnU9PLyfTvzHYYcuKQjES7dc/aVfKMxdAGFUU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QpW0DwuleymvyMEGSKfkyZ5NSkxg8xttyXSIoX94yR+t1JJP71wugT3asSQuwjEiX PcXYRYD+VcXqXIAak1/bjW/QwzzvY6eRp7NP0nhabXwbVMcUYcqnSg12swvm6xx2S5 YpeemKtO9QVSjHsYAA3DYHfnPdZugbOth1KuhmEU= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, shanzhulig , Maxime Ripard , Maarten Lankhorst , Thomas Zimmermann , David Airlie , stable@kernel.org, Daniel Vetter , Daniel Vetter , Linus Torvalds Subject: [PATCH 5.4 275/313] drm/atomic: Fix potential use-after-free in nonblocking commits Date: Tue, 25 Jul 2023 12:47:08 +0200 Message-ID: <20230725104532.969627955@linuxfoundation.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230725104521.167250627@linuxfoundation.org> References: <20230725104521.167250627@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Daniel Vetter commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. This requires a bit of background. Properly done a modeset driver's unload/remove sequence should be drm_dev_unplug(); drm_atomic_helper_shutdown(); drm_dev_put(); The trouble is that the drm_dev_unplugged() checks are by design racy, they do not synchronize against all outstanding ioctl. This is because those ioctl could block forever (both for modeset and for driver specific ioctls), leading to deadlocks in hotunplug. Instead the code sections that touch the hardware need to be annotated with drm_dev_enter/exit, to avoid accessing hardware resources after the unload/remove has finished. To avoid use-after-free issues all the involved userspace visible objects are supposed to hold a reference on the underlying drm_device, like drm_file does. The issue now is that we missed one, the atomic modeset ioctl can be run in a nonblocking fashion, and in that case it cannot rely on the implied drm_device reference provided by the ioctl calling context. This can result in a use-after-free if an nonblocking atomic commit is carefully raced against a driver unload. Fix this by unconditionally grabbing a drm_device reference for any drm_atomic_state structures. Strictly speaking this isn't required for blocking commits and TEST_ONLY calls, but it's the simpler approach. Thanks to shanzhulig for the initial idea of grabbing an unconditional reference, I just added comments, a condensed commit message and fixed a minor potential issue in where exactly we drop the final reference. Reported-by: shanzhulig Suggested-by: shanzhulig Reviewed-by: Maxime Ripard Cc: Maarten Lankhorst Cc: Thomas Zimmermann Cc: David Airlie Cc: stable@kernel.org Signed-off-by: Daniel Vetter Signed-off-by: Daniel Vetter Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -97,6 +97,12 @@ drm_atomic_state_init(struct drm_device if (!state->planes) goto fail; + /* + * Because drm_atomic_state can be committed asynchronously we need our + * own reference and cannot rely on the on implied by drm_file in the + * ioctl call. + */ + drm_dev_get(dev); state->dev = dev; DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state); @@ -256,7 +262,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); void __drm_atomic_state_free(struct kref *ref) { struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); - struct drm_mode_config *config = &state->dev->mode_config; + struct drm_device *dev = state->dev; + struct drm_mode_config *config = &dev->mode_config; drm_atomic_state_clear(state); @@ -268,6 +275,8 @@ void __drm_atomic_state_free(struct kref drm_atomic_state_default_release(state); kfree(state); } + + drm_dev_put(dev); } EXPORT_SYMBOL(__drm_atomic_state_free);