From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F379C30FA6 for ; Wed, 11 Oct 2023 16:04:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="Xc1EJqel" Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D7061B8 for ; Wed, 11 Oct 2023 09:04:01 -0700 (PDT) Received: by mail-pl1-x630.google.com with SMTP id d9443c01a7336-1c9d7a98abbso2739915ad.1 for ; Wed, 11 Oct 2023 09:04:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1697040241; x=1697645041; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=/JdPIzkZRw+UEDCwPct/StXsx1OeuztdzUWh/BOTJXs=; b=Xc1EJqelwNYwe6Kfm2eAtA+XjRix1DyZKgwQEYFbVIKUUCcIGcjZR2WLhB2MLXxPX0 94Q4uq4CfMxRvyn4xMGcNfiWykpv8hw1yYfq7Ti/9rQ58zqk4fpvoBPqYVp6aTd2cvZy PssJ3Cmh+uof7P87zQx6jkKvklsPgM5myBsKk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697040241; x=1697645041; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=/JdPIzkZRw+UEDCwPct/StXsx1OeuztdzUWh/BOTJXs=; b=MPTiAShXP1ow3+Y1ZGyDyNE1u5OXr+NBEhrE2ECRxqas2CAptRgu3U7kaQ1pZLyWvI akZUEZBP+QzJ5qRGcro532aVbhsMIJ/13IS8bLlqgTZdnivOaStd/0YZH5PuKpvqTjKh qrQOcx1R5Gv7ommVnGycnp7ZogwnExe7aTyu6j+Ja5Jsl2QeEgBRXCyJuJysajGXvwrO Vm5cDQzjeIaPxCTWA3w2kTEtOcf0vZPyq1YF9fIYyxnt/iuTg/u7edE7yaJaZ29IZUBj 98d0BO/wLtNmPlNlcK8ooAQ2WQCQNdoP/z7Zv91zlKPF3EXYyxHypMQFBc2nzX4l1j+R MNtQ== X-Gm-Message-State: AOJu0Yws/dvL1XTEG82Ir4S1JnIgSWU0W3f9RzxqT39hJ0KL/e5cn0tg HUhGFrDDNpPnrxMrBK+cXTnJhQ== X-Google-Smtp-Source: AGHT+IGqjWqrCIqSHnONJ5fjKcFYrHjY07tSyis9X/kZiXSoJrS/q64lZQJ+lrnayGHRVg3N8XWShA== X-Received: by 2002:a17:903:2289:b0:1c6:28f6:9545 with SMTP id b9-20020a170903228900b001c628f69545mr21755725plh.45.1697040241156; Wed, 11 Oct 2023 09:04:01 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id jw14-20020a170903278e00b001adf6b21c77sm9908plb.107.2023.10.11.09.04.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Oct 2023 09:04:00 -0700 (PDT) Date: Wed, 11 Oct 2023 09:03:58 -0700 From: Kees Cook To: "Gustavo A. R. Silva" Cc: Sumit Semwal , Gustavo Padovan , Christian =?iso-8859-1?Q?K=F6nig?= , Arvind Yadav , linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH][next] dma-buf: Fix NULL pointer dereference in dma_fence_enable_sw_signaling() Message-ID: <202310110903.FE533CBCD@keescook> References: Precedence: bulk X-Mailing-List: linux-hardening@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net On Wed, Oct 11, 2023 at 08:03:43AM -0600, Gustavo A. R. Silva wrote: > Currently, a NULL pointer dereference will happen in function > `dma_fence_enable_sw_signaling()` (at line 615), in case `chain` > is not allocated in `mock_chain()` and this function returns > `NULL` (at line 86). See below: > > drivers/dma-buf/st-dma-fence-chain.c: > 86 chain = mock_chain(NULL, f, 1); > 87 if (!chain) > 88 err = -ENOMEM; > 89 > 90 dma_fence_enable_sw_signaling(chain); Instead of the larger patch, should line 88 here just do a "return -ENOMEM" instead? -Kees > > drivers/dma-buf/dma-fence.c: > 611 void dma_fence_enable_sw_signaling(struct dma_fence *fence) > 612 { > 613 unsigned long flags; > 614 > 615 spin_lock_irqsave(fence->lock, flags); > ^^^^^^^^^^^ > | > NULL pointer reference > if fence == NULL > > 616 __dma_fence_enable_signaling(fence); > 617 spin_unlock_irqrestore(fence->lock, flags); > 618 } > > Fix this by adding a NULL check before dereferencing `fence` in > `dma_fence_enable_sw_signaling()`. This will prevent any other NULL > pointer dereference when the `fence` passed as an argument is `NULL`. > > Addresses-Coverity: ("Dereference after null check") > Fixes: d62c43a953ce ("dma-buf: Enable signaling on fence for selftests") > Cc: stable@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva > --- > drivers/dma-buf/dma-fence.c | 9 ++++++++- > include/linux/dma-fence.h | 2 +- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 8aa8f8cb7071..4d2f13560d0f 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -607,14 +607,21 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence) > * This will request for sw signaling to be enabled, to make the fence > * complete as soon as possible. This calls &dma_fence_ops.enable_signaling > * internally. > + * > + * Returns 0 on success and a negative error value when @fence is NULL. > */ > -void dma_fence_enable_sw_signaling(struct dma_fence *fence) > +int dma_fence_enable_sw_signaling(struct dma_fence *fence) > { > unsigned long flags; > > + if (!fence) > + return -EINVAL; > + > spin_lock_irqsave(fence->lock, flags); > __dma_fence_enable_signaling(fence); > spin_unlock_irqrestore(fence->lock, flags); > + > + return 0; > } > EXPORT_SYMBOL(dma_fence_enable_sw_signaling); > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index ebe78bd3d121..1e4025e925e6 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -399,7 +399,7 @@ int dma_fence_add_callback(struct dma_fence *fence, > dma_fence_func_t func); > bool dma_fence_remove_callback(struct dma_fence *fence, > struct dma_fence_cb *cb); > -void dma_fence_enable_sw_signaling(struct dma_fence *fence); > +int dma_fence_enable_sw_signaling(struct dma_fence *fence); > > /** > * dma_fence_is_signaled_locked - Return an indication if the fence > -- > 2.34.1 > > -- Kees Cook 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 E1129CD6E57 for ; Wed, 11 Oct 2023 16:04:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3FC6710E529; Wed, 11 Oct 2023 16:04:03 +0000 (UTC) Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) by gabe.freedesktop.org (Postfix) with ESMTPS id BC65E10E529 for ; Wed, 11 Oct 2023 16:04:01 +0000 (UTC) Received: by mail-pl1-x62e.google.com with SMTP id d9443c01a7336-1c0ecb9a075so47485385ad.2 for ; Wed, 11 Oct 2023 09:04:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1697040241; x=1697645041; darn=lists.freedesktop.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=/JdPIzkZRw+UEDCwPct/StXsx1OeuztdzUWh/BOTJXs=; b=FGz3P1PqjDjw3USmDdcKE8sCLOtEeTLLk/QqToWWko7/ixzBbxy/SoxGpOK9jz+M4/ GGNKviAvok9hRQJPFsxlhJoFHJDXbIhH5U7saGnEAOQQbjCarJ+CvGWlwt3MhVI/7Bkr 8PwrAXg1BjpQFUBfLxx0KnvGci1jxo04k55fM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697040241; x=1697645041; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=/JdPIzkZRw+UEDCwPct/StXsx1OeuztdzUWh/BOTJXs=; b=Ebki/smbFYEsOwrfaudXLiWLTiONxv1P5TYOPVZzIUBWe6ByAF0lv81XjQtE+hCuDV FGvXynmbddJECdzoyliLMYv4D8zC//jlyzcIrNO2PEE275AXdjN7LeSBKYYeL9pjTY1W 7nyN3C0dvp9pEXba47dTPhl7jusEfvuwMA6V6SFKkur+apeEivSAYrlf5DmSDldev5NY j23Ag4r2fzRBMYTc6uM1P5XN3hat2xCcK5nQ/HTZlmj1Ae9SPFFFZDvaSiOXZHEDo+ph 2uh/LQn6fgbYpvLWzZ3jVFGrqgHvacdEhc/36BAqthqB/UI6uAd5fuqHXjytlXoHSEDf PKrw== X-Gm-Message-State: AOJu0YyxlqkpyuiK/zk+4AAVgsvF/YrAYEO1AOxWoNCXgfTym4Oo3Di4 AWhb/DZ44rhnYOAEd0CwolYRfA== X-Google-Smtp-Source: AGHT+IGqjWqrCIqSHnONJ5fjKcFYrHjY07tSyis9X/kZiXSoJrS/q64lZQJ+lrnayGHRVg3N8XWShA== X-Received: by 2002:a17:903:2289:b0:1c6:28f6:9545 with SMTP id b9-20020a170903228900b001c628f69545mr21755725plh.45.1697040241156; Wed, 11 Oct 2023 09:04:01 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id jw14-20020a170903278e00b001adf6b21c77sm9908plb.107.2023.10.11.09.04.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Oct 2023 09:04:00 -0700 (PDT) Date: Wed, 11 Oct 2023 09:03:58 -0700 From: Kees Cook To: "Gustavo A. R. Silva" Subject: Re: [PATCH][next] dma-buf: Fix NULL pointer dereference in dma_fence_enable_sw_signaling() Message-ID: <202310110903.FE533CBCD@keescook> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: Gustavo Padovan , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Christian =?iso-8859-1?Q?K=F6nig?= , linaro-mm-sig@lists.linaro.org, linux-hardening@vger.kernel.org, Arvind Yadav , Sumit Semwal , linux-media@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, Oct 11, 2023 at 08:03:43AM -0600, Gustavo A. R. Silva wrote: > Currently, a NULL pointer dereference will happen in function > `dma_fence_enable_sw_signaling()` (at line 615), in case `chain` > is not allocated in `mock_chain()` and this function returns > `NULL` (at line 86). See below: > > drivers/dma-buf/st-dma-fence-chain.c: > 86 chain = mock_chain(NULL, f, 1); > 87 if (!chain) > 88 err = -ENOMEM; > 89 > 90 dma_fence_enable_sw_signaling(chain); Instead of the larger patch, should line 88 here just do a "return -ENOMEM" instead? -Kees > > drivers/dma-buf/dma-fence.c: > 611 void dma_fence_enable_sw_signaling(struct dma_fence *fence) > 612 { > 613 unsigned long flags; > 614 > 615 spin_lock_irqsave(fence->lock, flags); > ^^^^^^^^^^^ > | > NULL pointer reference > if fence == NULL > > 616 __dma_fence_enable_signaling(fence); > 617 spin_unlock_irqrestore(fence->lock, flags); > 618 } > > Fix this by adding a NULL check before dereferencing `fence` in > `dma_fence_enable_sw_signaling()`. This will prevent any other NULL > pointer dereference when the `fence` passed as an argument is `NULL`. > > Addresses-Coverity: ("Dereference after null check") > Fixes: d62c43a953ce ("dma-buf: Enable signaling on fence for selftests") > Cc: stable@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva > --- > drivers/dma-buf/dma-fence.c | 9 ++++++++- > include/linux/dma-fence.h | 2 +- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 8aa8f8cb7071..4d2f13560d0f 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -607,14 +607,21 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence) > * This will request for sw signaling to be enabled, to make the fence > * complete as soon as possible. This calls &dma_fence_ops.enable_signaling > * internally. > + * > + * Returns 0 on success and a negative error value when @fence is NULL. > */ > -void dma_fence_enable_sw_signaling(struct dma_fence *fence) > +int dma_fence_enable_sw_signaling(struct dma_fence *fence) > { > unsigned long flags; > > + if (!fence) > + return -EINVAL; > + > spin_lock_irqsave(fence->lock, flags); > __dma_fence_enable_signaling(fence); > spin_unlock_irqrestore(fence->lock, flags); > + > + return 0; > } > EXPORT_SYMBOL(dma_fence_enable_sw_signaling); > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index ebe78bd3d121..1e4025e925e6 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -399,7 +399,7 @@ int dma_fence_add_callback(struct dma_fence *fence, > dma_fence_func_t func); > bool dma_fence_remove_callback(struct dma_fence *fence, > struct dma_fence_cb *cb); > -void dma_fence_enable_sw_signaling(struct dma_fence *fence); > +int dma_fence_enable_sw_signaling(struct dma_fence *fence); > > /** > * dma_fence_is_signaled_locked - Return an indication if the fence > -- > 2.34.1 > > -- Kees Cook