* [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-18 15:41 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
@ 2024-09-18 15:41 ` Yonatan Maman
0 siblings, 0 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-09-18 15:41 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, Gal Shalom
A copy push command might fail, causing `migrate_to_ram` to return a
dirty HIGH_USER page to the user.
This exposes a security vulnerability in the nouveau driver. To prevent
memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
page for the destination page.
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 6fb65b01d778..097bd3af0719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
goto done;
- dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
+ dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
if (!dpage)
goto done;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 0/2] *** BUG Fix for Nouveau Memory***
@ 2024-09-23 13:54 Yonatan Maman
2024-09-23 13:54 ` [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel Yonatan Maman
2024-09-23 13:54 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
0 siblings, 2 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-09-23 13:54 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman
This patch series addresses two critical issues in the Nouveau driver related to device channels, error handling and memory leaking.
- Memory Leak in migrate_to_ram - the migrate_to_ram function was identified as leaking memory when a copy push command fails. This results in the function returning a dirty HIGH_USER page, which can expose sensitive information and pose a security risk. To mitigate this vulnerability, the patch ensures that a zero page is allocated for the destination page, thereby preventing memory leaks and enhancing the security of the driver in case of failure.
- Privileged Error in Copy Engine Channel - an error was observed when the nouveau_dmem_copy_one function is executed, leading to a Host Copy Engine Privileged error on channel 1. The patch addresses this by adjusting the Copy Engine channel configuration to permit privileged push commands, thus resolving the error.
Yonatan Maman (2):
nouveau/dmem: Fix privileged error in copy engine channel
nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel
2024-09-23 13:54 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
@ 2024-09-23 13:54 ` Yonatan Maman
2024-09-30 11:09 ` Danilo Krummrich
2024-09-23 13:54 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
1 sibling, 1 reply; 13+ messages in thread
From: Yonatan Maman @ 2024-09-23 13:54 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, Gal Shalom
When `nouveau_dmem_copy_one` is called, the following error occurs:
[272146.675156] nouveau 0000:06:00.0: fifo: PBDMA9: 00000004 [HCE_PRIV]
ch 1 00000300 00003386
This indicates that a copy push command triggered a Host Copy Engine
Privileged error on channel 1 (Copy Engine channel). To address this
issue, modify the Copy Engine channel to allow privileged push commands
Fixes: 6de125383a5cc
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index a58c31089613..0a75ce4c5021 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -356,7 +356,7 @@ nouveau_accel_ce_init(struct nouveau_drm *drm)
return;
}
- ret = nouveau_channel_new(drm, device, false, runm, NvDmaFB, NvDmaTT, &drm->cechan);
+ ret = nouveau_channel_new(drm, device, true, runm, NvDmaFB, NvDmaTT, &drm->cechan);
if (ret)
NV_ERROR(drm, "failed to create ce channel, %d\n", ret);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-23 13:54 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
2024-09-23 13:54 ` [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel Yonatan Maman
@ 2024-09-23 13:54 ` Yonatan Maman
2024-09-30 11:20 ` Danilo Krummrich
1 sibling, 1 reply; 13+ messages in thread
From: Yonatan Maman @ 2024-09-23 13:54 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, Gal Shalom
A copy push command might fail, causing `migrate_to_ram` to return a
dirty HIGH_USER page to the user.
This exposes a security vulnerability in the nouveau driver. To prevent
memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
page for the destination page.
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 6fb65b01d778..097bd3af0719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
goto done;
- dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
+ dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
if (!dpage)
goto done;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-29 11:44 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
@ 2024-09-29 11:44 ` Yonatan Maman
0 siblings, 0 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-09-29 11:44 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, Gal Shalom
From: Yonatan Maman <Ymaman@Nvidia.com>
A copy push command might fail, causing `migrate_to_ram` to return a
dirty HIGH_USER page to the user.
This exposes a security vulnerability in the nouveau driver. To prevent
memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
page for the destination page.
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 6fb65b01d778..097bd3af0719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
goto done;
- dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
+ dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
if (!dpage)
goto done;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-29 12:08 [PATCH 0/2] *** drm/nouveau/dmem: BUG Fixes for Nouveau DMEM*** Yonatan Maman
@ 2024-09-29 12:08 ` Yonatan Maman
0 siblings, 0 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-09-29 12:08 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, Gal Shalom
From: Yonatan Maman <Ymaman@Nvidia.com>
A copy push command might fail, causing `migrate_to_ram` to return a
dirty HIGH_USER page to the user.
This exposes a security vulnerability in the nouveau driver. To prevent
memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
page for the destination page.
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 6fb65b01d778..097bd3af0719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
goto done;
- dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
+ dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
if (!dpage)
goto done;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-29 12:16 [PATCH 0/2] *** drm/nouveau/dmem: Fix Memory Leaking and Device Channels configuration*** Yonatan Maman
@ 2024-09-29 12:16 ` Yonatan Maman
0 siblings, 0 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-09-29 12:16 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, Gal Shalom
From: Yonatan Maman <Ymaman@Nvidia.com>
A copy push command might fail, causing `migrate_to_ram` to return a
dirty HIGH_USER page to the user.
This exposes a security vulnerability in the nouveau driver. To prevent
memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
page for the destination page.
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 6fb65b01d778..097bd3af0719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
goto done;
- dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
+ dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
if (!dpage)
goto done;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-29 12:30 [PATCH 0/2] *** drm/nouveau/dmem: Fix Memory Leaking and Device Channels configuration*** Yonatan Maman
@ 2024-09-29 12:30 ` Yonatan Maman
0 siblings, 0 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-09-29 12:30 UTC (permalink / raw)
To: nouveau; +Cc: Yonatan Maman, apopple, bskeggs, Gal Shalom
From: Yonatan Maman <Ymaman@Nvidia.com>
A copy push command might fail, causing `migrate_to_ram` to return a
dirty HIGH_USER page to the user.
This exposes a security vulnerability in the nouveau driver. To prevent
memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
page for the destination page.
Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 6fb65b01d778..097bd3af0719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
goto done;
- dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
+ dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
if (!dpage)
goto done;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel
2024-09-23 13:54 ` [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel Yonatan Maman
@ 2024-09-30 11:09 ` Danilo Krummrich
2024-10-07 12:35 ` Yonatan Maman
0 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2024-09-30 11:09 UTC (permalink / raw)
To: Yonatan Maman; +Cc: nouveau, Gal Shalom
Hi Yonatan,
On Mon, Sep 23, 2024 at 01:54:56PM +0000, Yonatan Maman wrote:
> When `nouveau_dmem_copy_one` is called, the following error occurs:
>
> [272146.675156] nouveau 0000:06:00.0: fifo: PBDMA9: 00000004 [HCE_PRIV]
> ch 1 00000300 00003386
>
> This indicates that a copy push command triggered a Host Copy Engine
> Privileged error on channel 1 (Copy Engine channel). To address this
> issue, modify the Copy Engine channel to allow privileged push commands
>
> Fixes: 6de125383a5cc
> Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
> Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
Please read [1] and use scripts/checkpatch.pl and scripts/get_maintainer.pl
before sending patches.
In particular, the 'Fixes' tag has a defined format, I recommend:
```
[core]
abbrev = 12
[pretty]
fixes = Fixes: %h (\"%s\")
```
in your `.gitconfig`.
Also make sure so use 'Co-developed-by' if there is a co-author; I see that this
patch is also signed off by Gal Shalom.
Please also send the patches to all relevant mailing lists and maintainers to
avoid your patches not getting the required attention.
[1] https://docs.kernel.org/process/submitting-patches.html
> ---
> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index a58c31089613..0a75ce4c5021 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -356,7 +356,7 @@ nouveau_accel_ce_init(struct nouveau_drm *drm)
> return;
> }
>
> - ret = nouveau_channel_new(drm, device, false, runm, NvDmaFB, NvDmaTT, &drm->cechan);
> + ret = nouveau_channel_new(drm, device, true, runm, NvDmaFB, NvDmaTT, &drm->cechan);
> if (ret)
> NV_ERROR(drm, "failed to create ce channel, %d\n", ret);
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-23 13:54 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
@ 2024-09-30 11:20 ` Danilo Krummrich
2024-10-07 12:28 ` Yonatan Maman
0 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2024-09-30 11:20 UTC (permalink / raw)
To: Yonatan Maman; +Cc: nouveau, Gal Shalom
On Mon, Sep 23, 2024 at 01:54:58PM +0000, Yonatan Maman wrote:
> A copy push command might fail, causing `migrate_to_ram` to return a
> dirty HIGH_USER page to the user.
>
> This exposes a security vulnerability in the nouveau driver. To prevent
> memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
> page for the destination page.
So, you refer to the case where this function fails in nouveau_dmem_copy_one()?
If so, can you please explain why adding __GFP_ZERO to alloc_page_vma() helps
with that?
>
> Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
> Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
Since this is a bug, please also add a 'Fixes' tag, CC stable and add a
'Co-developed-by' tag if appropriate.
> ---
> drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> index 6fb65b01d778..097bd3af0719 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> @@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
> if (!spage || !(src & MIGRATE_PFN_MIGRATE))
> goto done;
>
> - dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
> + dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
> if (!dpage)
> goto done;
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-09-30 11:20 ` Danilo Krummrich
@ 2024-10-07 12:28 ` Yonatan Maman
2024-10-07 13:51 ` Danilo Krummrich
0 siblings, 1 reply; 13+ messages in thread
From: Yonatan Maman @ 2024-10-07 12:28 UTC (permalink / raw)
To: Danilo Krummrich
Cc: nouveau, Gal Shalom, kherbst, lyude, dakr, airlied, daniel,
dri-devel, nouveau, linux-kernel
On 30/09/2024 14:20, Danilo Krummrich wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Sep 23, 2024 at 01:54:58PM +0000, Yonatan Maman wrote:
>> A copy push command might fail, causing `migrate_to_ram` to return a
>> dirty HIGH_USER page to the user.
>>
>> This exposes a security vulnerability in the nouveau driver. To prevent
>> memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
>> page for the destination page.
>
> So, you refer to the case where this function fails in nouveau_dmem_copy_one()?
>
> If so, can you please explain why adding __GFP_ZERO to alloc_page_vma() helps
> with that?
>
The nouveau_dmem_copy_one function ensures that the copy push command is
sent to the device firmware but does not track whether it was executed
successfully.
In the case of a copy error (e.g., firmware or hardware error), the
command will be sent in the firmware channel, and nouveau_dmem_copy_one
might succeed, as well as the migrate_to_ram function. Thus, a dirty
page could be returned to the user.
It’s important to note that we attempted to use nouveau_fence_wait
status to handle migration errors, but it does not catch all error types.
To avoid this vulnerability, we allocate a zero page. So that, in case
of an error, a non-dirty (zero) page will be returned to the user.
>>
>> Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
>> Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
>
> Since this is a bug, please also add a 'Fixes' tag, CC stable and add a
> 'Co-developed-by' tag if appropriate.
sure, thanks, I will add, and push it as V2 patch-series.
>
>> ---
>> drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
>> index 6fb65b01d778..097bd3af0719 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
>> @@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
>> if (!spage || !(src & MIGRATE_PFN_MIGRATE))
>> goto done;
>>
>> - dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
>> + dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
>> if (!dpage)
>> goto done;
>>
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel
2024-09-30 11:09 ` Danilo Krummrich
@ 2024-10-07 12:35 ` Yonatan Maman
0 siblings, 0 replies; 13+ messages in thread
From: Yonatan Maman @ 2024-10-07 12:35 UTC (permalink / raw)
To: Danilo Krummrich
Cc: nouveau, Gal Shalom, kherbst, lyude, dakr, airlied, daniel,
dri-devel, linux-kernel
On 30/09/2024 14:09, Danilo Krummrich wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Yonatan,
>
> On Mon, Sep 23, 2024 at 01:54:56PM +0000, Yonatan Maman wrote:
>> When `nouveau_dmem_copy_one` is called, the following error occurs:
>>
>> [272146.675156] nouveau 0000:06:00.0: fifo: PBDMA9: 00000004 [HCE_PRIV]
>> ch 1 00000300 00003386
>>
>> This indicates that a copy push command triggered a Host Copy Engine
>> Privileged error on channel 1 (Copy Engine channel). To address this
>> issue, modify the Copy Engine channel to allow privileged push commands
>>
>> Fixes: 6de125383a5cc
>> Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
>> Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
>
> Please read [1] and use scripts/checkpatch.pl and scripts/get_maintainer.pl
> before sending patches.
>
> In particular, the 'Fixes' tag has a defined format, I recommend:
>
> ```
> [core]
> abbrev = 12
> [pretty]
> fixes = Fixes: %h (\"%s\")
> ```
>
> in your `.gitconfig`.
>
> Also make sure so use 'Co-developed-by' if there is a co-author; I see that this
> patch is also signed off by Gal Shalom.
>
> Please also send the patches to all relevant mailing lists and maintainers to
> avoid your patches not getting the required attention.
>
> [1] https://docs.kernel.org/process/submitting-patches.html
Thanks for the feedback! I will address these comments and send V2
patch-set.
>
>> ---
>> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> index a58c31089613..0a75ce4c5021 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> @@ -356,7 +356,7 @@ nouveau_accel_ce_init(struct nouveau_drm *drm)
>> return;
>> }
>>
>> - ret = nouveau_channel_new(drm, device, false, runm, NvDmaFB, NvDmaTT, &drm->cechan);
>> + ret = nouveau_channel_new(drm, device, true, runm, NvDmaFB, NvDmaTT, &drm->cechan);
>> if (ret)
>> NV_ERROR(drm, "failed to create ce channel, %d\n", ret);
>> }
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error
2024-10-07 12:28 ` Yonatan Maman
@ 2024-10-07 13:51 ` Danilo Krummrich
0 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2024-10-07 13:51 UTC (permalink / raw)
To: Yonatan Maman
Cc: nouveau, Gal Shalom, kherbst, lyude, dakr, airlied, daniel,
dri-devel, linux-kernel
On Mon, Oct 07, 2024 at 03:28:22PM +0300, Yonatan Maman wrote:
>
>
> On 30/09/2024 14:20, Danilo Krummrich wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Mon, Sep 23, 2024 at 01:54:58PM +0000, Yonatan Maman wrote:
> > > A copy push command might fail, causing `migrate_to_ram` to return a
> > > dirty HIGH_USER page to the user.
> > >
> > > This exposes a security vulnerability in the nouveau driver. To prevent
> > > memory leaks in `migrate_to_ram` upon a copy error, allocate a zero
> > > page for the destination page.
> >
> > So, you refer to the case where this function fails in nouveau_dmem_copy_one()?
> >
> > If so, can you please explain why adding __GFP_ZERO to alloc_page_vma() helps
> > with that?
> >
>
> The nouveau_dmem_copy_one function ensures that the copy push command is
> sent to the device firmware but does not track whether it was executed
> successfully.
>
> In the case of a copy error (e.g., firmware or hardware error), the command
> will be sent in the firmware channel, and nouveau_dmem_copy_one might
> succeed, as well as the migrate_to_ram function. Thus, a dirty page could be
> returned to the user.
>
> It’s important to note that we attempted to use nouveau_fence_wait status to
> handle migration errors, but it does not catch all error types.
>
> To avoid this vulnerability, we allocate a zero page. So that, in case of an
> error, a non-dirty (zero) page will be returned to the user.
I see, I got confused by calling this a 'memory leak'.
Please add this description in the commit message and avoid the term 'memory
leak' in this context.
>
> > >
> > > Signed-off-by: Yonatan Maman <Ymaman@Nvidia.com>
> > > Signed-off-by: Gal Shalom <GalShalom@Nvidia.com>
> >
> > Since this is a bug, please also add a 'Fixes' tag, CC stable and add a
> > 'Co-developed-by' tag if appropriate.
>
> sure, thanks, I will add, and push it as V2 patch-series.
> >
> > > ---
> > > drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> > > index 6fb65b01d778..097bd3af0719 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> > > @@ -193,7 +193,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
> > > if (!spage || !(src & MIGRATE_PFN_MIGRATE))
> > > goto done;
> > >
> > > - dpage = alloc_page_vma(GFP_HIGHUSER, vmf->vma, vmf->address);
> > > + dpage = alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO, vmf->vma, vmf->address);
> > > if (!dpage)
> > > goto done;
> > >
> > > --
> > > 2.34.1
> > >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-10-07 13:51 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-23 13:54 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
2024-09-23 13:54 ` [PATCH 1/2] nouveau/dmem: Fix privileged error in copy engine channel Yonatan Maman
2024-09-30 11:09 ` Danilo Krummrich
2024-10-07 12:35 ` Yonatan Maman
2024-09-23 13:54 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
2024-09-30 11:20 ` Danilo Krummrich
2024-10-07 12:28 ` Yonatan Maman
2024-10-07 13:51 ` Danilo Krummrich
-- strict thread matches above, loose matches on Subject: below --
2024-09-29 12:30 [PATCH 0/2] *** drm/nouveau/dmem: Fix Memory Leaking and Device Channels configuration*** Yonatan Maman
2024-09-29 12:30 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
2024-09-29 12:16 [PATCH 0/2] *** drm/nouveau/dmem: Fix Memory Leaking and Device Channels configuration*** Yonatan Maman
2024-09-29 12:16 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
2024-09-29 12:08 [PATCH 0/2] *** drm/nouveau/dmem: BUG Fixes for Nouveau DMEM*** Yonatan Maman
2024-09-29 12:08 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
2024-09-29 11:44 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
2024-09-29 11:44 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
2024-09-18 15:41 [PATCH 0/2] *** BUG Fix for Nouveau Memory*** Yonatan Maman
2024-09-18 15:41 ` [PATCH 2/2] nouveau/dmem: Fix memory leak in `migrate_to_ram` upon copy error Yonatan Maman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.