Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
From:   Yuran Pereira <yuran.pereira@hotmail.com>
To:     airlied@gmail.com
Cc:     Yuran Pereira <yuran.pereira@hotmail.com>, kherbst@redhat.com,
        lyude@redhat.com, dakr@redhat.com, daniel@ffwll.ch,
        sumit.semwal@linaro.org, christian.koenig@amd.com,
        dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
        linaro-mm-sig@lists.linaro.org,
        linux-kernel-mentees@lists.linuxfoundation.org
Subject: [PATCH] drm/nouveau: Prevents NULL pointer dereference in nouveau_uvmm_sm_prepare
Date:   Thu, 26 Oct 2023 22:33:40 +0530
Message-ID: <DB3PR10MB6835FA6E15F3C830FC793D2EE8DDA@DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: DB3PR10MB6835:EE_|AM0PR10MB3156:EE_
X-MS-Office365-Filtering-Correlation-Id: f895dcd2-29f6-4ae9-586d-08dbd6459038
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: 4hf+MRy1VZqH34l9xHxRby1/Pdz7Ra2vKqsumcGu/7mLG0OJD9b/5vmGOKNmQbD12TgbxOx4cD+IY8emY+8fer6LTwWDafTKsQk5wI90QNvuzCtcoUDxxaFGkmxAHkwgKRVucY89JUGaKu+/rJ/CUx7WjjKshzTczo2IinSyZfacJ1nCf2RKx1bC76ohEUaS7WuiQvAdWz6rAwrY9hND/zzb9rrfvKU3se2fSwgCnRRduwM5nNLylw47wfqIo00huyWWqYohiARHzJUVghgdedy1cZfKIkP4PDYcTclcYArYUrftJeImWw/u86zUcwD7wi/A8w7LZ+JWrhq7nlohA8ezYIHdVQo/MbWusN2iQoK5Kz8GXglcdm2vjuIaUFl1mKiBSYYhmoj/f/b+u7JXf+EpLQYjx9g0QBH5LHT3M+p5Q/091/1SqRicMDHFr8Tz2y0VfZba7mymMkHDAhpSwenvct96YPAZZQYu5StbcEVNmMkkh4QVmNVV5o7HhLoxR6oH8EvrniDZcUd38aLdaqE7ctAceU+O80o6l+Xoh3Hd8wiBeq0Xo9XGAXOJAtXDpBwudlZvsiOE9iWJ+Xi03mYoYDdbE9Ht07KbrO0ntWQLAtywjabgFzk9vja3McZS
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?a97BNOaUMumDoLUEiNtfOO6f1zfrvCzeExZHbuHXIzi7eRp2PiVFDlI0EKbH?=
 =?us-ascii?Q?oauibER/ofwErc6oZmI/jJD7Hq5//9obUYe9GJdVCvFLpZJUyoYq8ILgDXgI?=
 =?us-ascii?Q?pF7fsGxDstR/9pAVWLLMYjosw7FOxyqyLYTWlqVLGSwsjpl+TmvK3nOtIfWc?=
 =?us-ascii?Q?HpXOozcbEH48xMr7UKa+kErmDlrX7Yio5pQMLwyYfPiLQoexfOme4MFkj8TC?=
 =?us-ascii?Q?YdpYyRVmki7ZRXZATLMxgaMRW3OJvDGtb6PhphZdfaVDJ2JIDJJ9U6f7GDOT?=
 =?us-ascii?Q?rdhGu4nb2IlnWj+goQPt3c7SQ8aZsHRkbWYFILgOHj003bSpsGOlm+JBsNno?=
 =?us-ascii?Q?Ykvusgaoq+g3kbhc4qjF/E4C++iapsdXwWnsuwLn9hWX3ZcfDVU4YXUFhn7S?=
 =?us-ascii?Q?TSj8IRemeTx0Scy5V8uukbv/pyhuXthQadRQzXB9Sj5/Hw9xPBgg45N6aBWW?=
 =?us-ascii?Q?yJw9bxu+nw1Arth1rNfmDN6z1B+f96dUZdSi4rsQPc+YcwZX39/Xx/fwT/l2?=
 =?us-ascii?Q?6lXEl4H3PSomxBNuYewBTjg3qo+4JDDNBTdE8a9hHOC1A4wz2ajjXYzdHjTO?=
 =?us-ascii?Q?fNeKXBL23ou1hSmvDX3kXRsS3mMxWDAiSE8h91Gc+xr9p0MdkUvSgzktaBk3?=
 =?us-ascii?Q?dOf33ynFT4C5mXjR4Je4HAGBVU73M4voUYIxc/VuW7xznaMtp9X626qk/lpK?=
 =?us-ascii?Q?lTKwM8S5awzcchnLpEA2AD3G4PNKe1XjT2Kig9U8CSxRADlm8eaS3a8iHaLg?=
 =?us-ascii?Q?lIxzQluq5hp9jd3vfPX50csY8ndmWxRgy4YvfVBPvcQfo5kKDvln90lX/CRt?=
 =?us-ascii?Q?nqhWqdzo7IuLDkyczGj5e0o6zgedsEunjtbD7bdgkW6bkSEKOxFz/EIRoKvc?=
 =?us-ascii?Q?njgO6vMIwMsDT2qU6l7jLIoNw61bqMxvi9Irha50k1IDEJqWUtprInwDeFBm?=
 =?us-ascii?Q?vDxSMbsnIKoegfCt6A7JhAYFwNtWLjCBvzEzaL538pA2oUHm6fpEuth3cqFu?=
 =?us-ascii?Q?33F4f2nsZCxowj2j8ECE/0cSRfbbUUaj9kqWnglFbB4FRX5C62eGSziWt5Bs?=
 =?us-ascii?Q?Y0f+ANi3avufoFBSI5ixRuLJqlABMecgfzvv8YWmeJ9HEwIDNhl0WMPvJIIT?=
 =?us-ascii?Q?mVPwk5Ct/8IFSH8zyg+HM3CQhoEcMn29Ok8XMC0USFnqDuMlKBVkZQep2ELr?=
 =?us-ascii?Q?MjONvzPdgQs4kwCOrdiAEkXyvJFjVm2rQOkxwBqUtKRVdnaUEK/7gg84goo?=
 =?us-ascii?Q?=3D?=
X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-6b909.templateTenant
X-MS-Exchange-CrossTenant-Network-Message-Id: f895dcd2-29f6-4ae9-586d-08dbd6459038
X-MS-Exchange-CrossTenant-AuthSource: DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 17:04:06.7700
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR10MB3156
X-Spam-Status: No, score=0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,FORGED_HOTMAIL_RCVD2,FREEMAIL_FORGED_FROMDOMAIN,
	FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Thu, 26 Oct 2023 10:04:37 -0700 (PDT)

There are instances where the "args" argument passed to
nouveau_uvmm_sm_prepare() is NULL.

I.e. when nouveau_uvmm_sm_prepare() is called from
nouveau_uvmm_sm_unmap_prepare()

```
static int
nouveau_uvmm_sm_unmap_prepare(struct nouveau_uvmm *uvmm,
...
{
    return nouveau_uvmm_sm_prepare(uvmm, new, ops, NULL);
}
```

The problem is that op_map_prepare() which nouveau_uvmm_sm_prepare
calls, dereferences this value, which can lead to a NULL pointer
dereference.

```
static int
op_map_prepare(struct nouveau_uvmm *uvmm,
...
{
    ...
    uvma->region = args->region; <-- Dereferencing of possibly NULL pointer
    uvma->kind = args->kind;     <--
    ...
}
```

```
static int
nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
...
            struct uvmm_map_args *args)
{
    struct drm_gpuva_op *op;
    u64 vmm_get_start = args ? args->addr : 0;
    u64 vmm_get_end = args ? args->addr + args->range : 0;
    int ret;

    drm_gpuva_for_each_op(op, ops) {
        switch (op->op) {
        case DRM_GPUVA_OP_MAP: {
            u64 vmm_get_range = vmm_get_end - vmm_get_start;

            ret = op_map_prepare(uvmm, &new->map, &op->map, args); <---
            if (ret)
                goto unwind;

            if (args && vmm_get_range) {
                ret = nouveau_uvmm_vmm_get(uvmm, vmm_get_start,
                               vmm_get_range);
                if (ret) {
                    op_map_prepare_unwind(new->map);
                    goto unwind;
                }
            }
    ...
```

Since the switch "case DRM_GPUVA_OP_MAP", also NULL checks "args"
after the call to op_map_prepare(), my guess is that we should
probably relocate this check to a point before op_map_prepare()
is called.

This patch ensures that the value of args is checked before
calling op_map_prepare()

Addresses-Coverity-ID: 1544574 ("Dereference after null check")
Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index aae780e4a4aa..6baa481eb2c8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -620,11 +620,14 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 		case DRM_GPUVA_OP_MAP: {
 			u64 vmm_get_range = vmm_get_end - vmm_get_start;
 
+			if (!args)
+				goto unwind;
+
 			ret = op_map_prepare(uvmm, &new->map, &op->map, args);
 			if (ret)
 				goto unwind;
 
-			if (args && vmm_get_range) {
+			if (vmm_get_range) {
 				ret = nouveau_uvmm_vmm_get(uvmm, vmm_get_start,
 							   vmm_get_range);
 				if (ret) {
-- 
2.25.1