Re: [PATCH] drm/xe: Declare __xe_lrc_*_ggtt_addr with __maybe__unused
Hi,
On Sun, Feb 04, 2024 at 02:23:24PM +0800, Dawei Li wrote:
> Kernel test robot reports building error:
>
> drivers/gpu/drm/xe/xe_lrc.c:544:1: error: unused function
> '__xe_lrc_regs_ggtt_addr' [-Werror,-Wunused-function]
> 544 | DECL_MAP_ADDR_HELPERS(regs)
> | ^~~
>
> drivers/gpu/drm/xe/xe_lrc.c:536:19: note: expanded from macro
> 'DECL_MAP_ADDR_HELPERS'
> 536 | static inline u32 __xe_lrc_##elem##_ggtt_addr(struct xe_lrc *lrc) \
> | ^~~
>
> :54:1: note: expanded from here
> 54 | __xe_lrc_regs_ggtt_addr
>| ^~~
>
> 1 error generated.
>
> Declare __xe_lrc_*_ggtt_addr with __maybe_unused to address it.
>
> Reported-by: kernel test robot
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202402010928.g3j2asbl-...@intel.com/
> Signed-off-by: Dawei Li
> ---
> drivers/gpu/drm/xe/xe_lrc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Just a gentle ping.
Thanks,
Dawei
>
> diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> index 0ec5ad2539f1..f70e60a2f8a3 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.c
> +++ b/drivers/gpu/drm/xe/xe_lrc.c
> @@ -614,7 +614,7 @@ static inline struct iosys_map
> __xe_lrc_##elem##_map(struct xe_lrc *lrc) \
> iosys_map_incr(, __xe_lrc_##elem##_offset(lrc)); \
> return map; \
> } \
> -static inline u32 __xe_lrc_##elem##_ggtt_addr(struct xe_lrc *lrc) \
> +static inline u32 __maybe_unused __xe_lrc_##elem##_ggtt_addr(struct xe_lrc
> *lrc) \
> { \
> return xe_bo_ggtt_addr(lrc->bo) + __xe_lrc_##elem##_offset(lrc); \
> } \
> --
> 2.27.0
>
[PATCH] drm/xe: Declare __xe_lrc_*_ggtt_addr with __maybe__unused
Kernel test robot reports building error:
drivers/gpu/drm/xe/xe_lrc.c:544:1: error: unused function
'__xe_lrc_regs_ggtt_addr' [-Werror,-Wunused-function]
544 | DECL_MAP_ADDR_HELPERS(regs)
| ^~~
drivers/gpu/drm/xe/xe_lrc.c:536:19: note: expanded from macro
'DECL_MAP_ADDR_HELPERS'
536 | static inline u32 __xe_lrc_##elem##_ggtt_addr(struct xe_lrc *lrc) \
| ^~~
:54:1: note: expanded from here
54 | __xe_lrc_regs_ggtt_addr
| ^~~
1 error generated.
Declare __xe_lrc_*_ggtt_addr with __maybe_unused to address it.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202402010928.g3j2asbl-...@intel.com/
Signed-off-by: Dawei Li
---
drivers/gpu/drm/xe/xe_lrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
index 0ec5ad2539f1..f70e60a2f8a3 100644
--- a/drivers/gpu/drm/xe/xe_lrc.c
+++ b/drivers/gpu/drm/xe/xe_lrc.c
@@ -614,7 +614,7 @@ static inline struct iosys_map __xe_lrc_##elem##_map(struct
xe_lrc *lrc) \
iosys_map_incr(, __xe_lrc_##elem##_offset(lrc)); \
return map; \
} \
-static inline u32 __xe_lrc_##elem##_ggtt_addr(struct xe_lrc *lrc) \
+static inline u32 __maybe_unused __xe_lrc_##elem##_ggtt_addr(struct xe_lrc
*lrc) \
{ \
return xe_bo_ggtt_addr(lrc->bo) + __xe_lrc_##elem##_offset(lrc); \
} \
--
2.27.0
Re: [PATCH v3] drm/vmwgfx: Fix race issue calling pin_user_pages
On Wed, Nov 09, 2022 at 11:37:34PM +0800, Dawei Li wrote:
> pin_user_pages() is unsafe without protection of mmap_lock,
> fix it by calling pin_user_pages_fast().
>
> Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-stats")
> Signed-off-by: Dawei Li
> ---
> v1:
> https://lore.kernel.org/all/tycp286mb23235c9a9fcf85c045f95ea7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
>
> v1->v2:
> Rebased to latest vmwgfx/drm-misc-fixes.
>
> v2->v3
> Replace pin_user_pages() with pin_user_pages_fast().
Gentle ping
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> index 089046fa21be..50fa3df0bc0c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> @@ -1085,21 +1085,21 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev,
> void *data,
> reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
>
> /* Pin mksGuestStat user pages and store those in the instance
> descriptor */
> - nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat,
> FOLL_LONGTERM, pages_stat, NULL);
> + nr_pinned_stat = pin_user_pages_fast(arg->stat, num_pages_stat,
> FOLL_LONGTERM, pages_stat);
> if (num_pages_stat != nr_pinned_stat)
> goto err_pin_stat;
>
> for (i = 0; i < num_pages_stat; ++i)
> pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
>
> - nr_pinned_info = pin_user_pages(arg->info, num_pages_info,
> FOLL_LONGTERM, pages_info, NULL);
> + nr_pinned_info = pin_user_pages_fast(arg->info, num_pages_info,
> FOLL_LONGTERM, pages_info);
> if (num_pages_info != nr_pinned_info)
> goto err_pin_info;
>
> for (i = 0; i < num_pages_info; ++i)
> pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
>
> - nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs,
> FOLL_LONGTERM, pages_strs, NULL);
> + nr_pinned_strs = pin_user_pages_fast(arg->strs, num_pages_strs,
> FOLL_LONGTERM, pages_strs);
> if (num_pages_strs != nr_pinned_strs)
> goto err_pin_strs;
>
> --
> 2.25.1
>
Re: [PATCH v4] dma-buf: fix racing conflict of dma_heap_add()
On Sat, Nov 05, 2022 at 12:05:36AM +0800, Dawei Li wrote:
Hi Christian,
May I have your opinion on this change?
Thanks,
Dawei
> Racing conflict could be:
> task A task B
> list_for_each_entry
> strcmp(h->name))
>list_for_each_entry
>strcmp(h->name)
> kzallockzalloc
> .. .
> device_create device_create
> list_add
>list_add
>
> The root cause is that task B has no idea about the fact someone
> else(A) has inserted heap with same name when it calls list_add,
> so a potential collision occurs.
>
> Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
> Signed-off-by: Dawei Li
> ---
> v1:
> https://lore.kernel.org/all/tycp286mb2323950197f60fc3473123b7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
> v1->v2: Narrow down locking scope, check the existence of heap before
> insertion, as suggested by Andrew Davis.
> v2->v3: Remove double checking.
> v3->v4: Minor coding style and patch formatting adjustment.
> ---
> drivers/dma-buf/dma-heap.c | 28 +++-
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 8f5848aa144f..59d158873f4c 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -233,18 +233,6 @@ struct dma_heap *dma_heap_add(const struct
> dma_heap_export_info *exp_info)
> return ERR_PTR(-EINVAL);
> }
>
> - /* check the name is unique */
> - mutex_lock(_list_lock);
> - list_for_each_entry(h, _list, list) {
> - if (!strcmp(h->name, exp_info->name)) {
> - mutex_unlock(_list_lock);
> - pr_err("dma_heap: Already registered heap named %s\n",
> -exp_info->name);
> - return ERR_PTR(-EINVAL);
> - }
> - }
> - mutex_unlock(_list_lock);
> -
> heap = kzalloc(sizeof(*heap), GFP_KERNEL);
> if (!heap)
> return ERR_PTR(-ENOMEM);
> @@ -283,13 +271,27 @@ struct dma_heap *dma_heap_add(const struct
> dma_heap_export_info *exp_info)
> err_ret = ERR_CAST(dev_ret);
> goto err2;
> }
> - /* Add heap to the list */
> +
> mutex_lock(_list_lock);
> + /* check the name is unique */
> + list_for_each_entry(h, _list, list) {
> + if (!strcmp(h->name, exp_info->name)) {
> + mutex_unlock(_list_lock);
> + pr_err("dma_heap: Already registered heap named %s\n",
> +exp_info->name);
> + err_ret = ERR_PTR(-EINVAL);
> + goto err3;
> + }
> + }
> +
> + /* Add heap to the list */
> list_add(>list, _list);
> mutex_unlock(_list_lock);
>
> return heap;
>
> +err3:
> + device_destroy(dma_heap_class, heap->heap_devt);
> err2:
> cdev_del(>heap_cdev);
> err1:
> --
> 2.25.1
>
[PATCH] drm/radeon: fix potential racing issue due to mmap_lock
Both find_vma() and get_user_pages() need explicit protection of
mmap lock, fix them by mmap_lock and get_user_pages_fast().
Fixes: ddd00e33e17a ("drm/radeon: add userptr flag to limit it to anonymous
memory v2")
Fixes: f72a113a71ab ("drm/radeon: add userptr support v8")
Signed-off-by: Dawei Li
---
drivers/gpu/drm/radeon/radeon_ttm.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index d33fec488713..741ea64b9402 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -351,7 +351,10 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_device
*bdev, struct ttm_tt *ttm
to prevent problems with writeback */
unsigned long end = gtt->userptr + (u64)ttm->num_pages *
PAGE_SIZE;
struct vm_area_struct *vma;
+
+ mmap_read_lock(gtt->usermm);
vma = find_vma(gtt->usermm, gtt->userptr);
+ mmap_read_unlock(gtt->usermm);
if (!vma || vma->vm_file || vma->vm_end < end)
return -EPERM;
}
@@ -361,8 +364,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_device
*bdev, struct ttm_tt *ttm
uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE;
struct page **pages = ttm->pages + pinned;
- r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0,
- pages, NULL);
+ r = get_user_pages_fast(userptr, num_pages, write ? FOLL_WRITE
: 0, pages);
if (r < 0)
goto release_pages;
--
2.25.1
[PATCH v3] drm/vmwgfx: Fix race issue calling pin_user_pages
pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling pin_user_pages_fast().
Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-stats")
Signed-off-by: Dawei Li
---
v1:
https://lore.kernel.org/all/tycp286mb23235c9a9fcf85c045f95ea7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2:
Rebased to latest vmwgfx/drm-misc-fixes.
v2->v3
Replace pin_user_pages() with pin_user_pages_fast().
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 089046fa21be..50fa3df0bc0c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1085,21 +1085,21 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
/* Pin mksGuestStat user pages and store those in the instance
descriptor */
- nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat, NULL);
+ nr_pinned_stat = pin_user_pages_fast(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat);
if (num_pages_stat != nr_pinned_stat)
goto err_pin_stat;
for (i = 0; i < num_pages_stat; ++i)
pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
- nr_pinned_info = pin_user_pages(arg->info, num_pages_info,
FOLL_LONGTERM, pages_info, NULL);
+ nr_pinned_info = pin_user_pages_fast(arg->info, num_pages_info,
FOLL_LONGTERM, pages_info);
if (num_pages_info != nr_pinned_info)
goto err_pin_info;
for (i = 0; i < num_pages_info; ++i)
pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
- nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs,
FOLL_LONGTERM, pages_strs, NULL);
+ nr_pinned_strs = pin_user_pages_fast(arg->strs, num_pages_strs,
FOLL_LONGTERM, pages_strs);
if (num_pages_strs != nr_pinned_strs)
goto err_pin_strs;
--
2.25.1
[PATCH v2] drm/vmwgfx: Protect pin_user_pages with mmap_lock
This patch includes changes below:
1) pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling mmap_read_lock() & mmap_read_unlock().
2) fix & refactor the incorrect exception handling procedure in
vmw_mksstat_add_ioctl().
Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-stats")
Signed-off-by: Dawei Li
---
v1:
https://lore.kernel.org/all/tycp286mb23235c9a9fcf85c045f95ea7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2:
Rebased to latest vmwgfx/drm-misc-fixes
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 089046fa21be..ec40a3364e0a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1020,9 +1020,9 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
const size_t num_pages_info = PFN_UP(arg->info_len);
const size_t num_pages_strs = PFN_UP(arg->strs_len);
long desc_len;
- long nr_pinned_stat;
- long nr_pinned_info;
- long nr_pinned_strs;
+ long nr_pinned_stat = 0;
+ long nr_pinned_info = 0;
+ long nr_pinned_strs = 0;
struct page *pages_stat[ARRAY_SIZE(pdesc->statPPNs)];
struct page *pages_info[ARRAY_SIZE(pdesc->infoPPNs)];
struct page *pages_strs[ARRAY_SIZE(pdesc->strsPPNs)];
@@ -1084,28 +1084,33 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
reset_ppn_array(pdesc->infoPPNs, ARRAY_SIZE(pdesc->infoPPNs));
reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
+ /* pin_user_pages() needs protection of mmap_lock */
+ mmap_read_lock(current->mm);
+
/* Pin mksGuestStat user pages and store those in the instance
descriptor */
nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat, NULL);
if (num_pages_stat != nr_pinned_stat)
- goto err_pin_stat;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_stat; ++i)
pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
nr_pinned_info = pin_user_pages(arg->info, num_pages_info,
FOLL_LONGTERM, pages_info, NULL);
if (num_pages_info != nr_pinned_info)
- goto err_pin_info;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_info; ++i)
pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs,
FOLL_LONGTERM, pages_strs, NULL);
if (num_pages_strs != nr_pinned_strs)
- goto err_pin_strs;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_strs; ++i)
pdesc->strsPPNs[i] = page_to_pfn(pages_strs[i]);
+ mmap_read_unlock(current->mm);
+
/* Send the descriptor to the host via a hypervisor call. The
mksGuestStat
pages will remain in use until the user requests a matching remove
stats
or a stats reset occurs. */
@@ -1120,15 +1125,15 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
return 0;
-err_pin_strs:
+__err_pin_pages:
+ mmap_read_unlock(current->mm);
+
if (nr_pinned_strs > 0)
unpin_user_pages(pages_strs, nr_pinned_strs);
-err_pin_info:
if (nr_pinned_info > 0)
unpin_user_pages(pages_info, nr_pinned_info);
-err_pin_stat:
if (nr_pinned_stat > 0)
unpin_user_pages(pages_stat, nr_pinned_stat);
--
2.25.1
[PATCH v4] dma-buf: fix racing conflict of dma_heap_add()
Racing conflict could be:
task A task B
list_for_each_entry
strcmp(h->name))
list_for_each_entry
strcmp(h->name)
kzallockzalloc
.. .
device_create device_create
list_add
list_add
The root cause is that task B has no idea about the fact someone
else(A) has inserted heap with same name when it calls list_add,
so a potential collision occurs.
Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
Signed-off-by: Dawei Li
---
v1:
https://lore.kernel.org/all/tycp286mb2323950197f60fc3473123b7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2: Narrow down locking scope, check the existence of heap before
insertion, as suggested by Andrew Davis.
v2->v3: Remove double checking.
v3->v4: Minor coding style and patch formatting adjustment.
---
drivers/dma-buf/dma-heap.c | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..59d158873f4c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -233,18 +233,6 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
return ERR_PTR(-EINVAL);
}
- /* check the name is unique */
- mutex_lock(_list_lock);
- list_for_each_entry(h, _list, list) {
- if (!strcmp(h->name, exp_info->name)) {
- mutex_unlock(_list_lock);
- pr_err("dma_heap: Already registered heap named %s\n",
- exp_info->name);
- return ERR_PTR(-EINVAL);
- }
- }
- mutex_unlock(_list_lock);
-
heap = kzalloc(sizeof(*heap), GFP_KERNEL);
if (!heap)
return ERR_PTR(-ENOMEM);
@@ -283,13 +271,27 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
err_ret = ERR_CAST(dev_ret);
goto err2;
}
- /* Add heap to the list */
+
mutex_lock(_list_lock);
+ /* check the name is unique */
+ list_for_each_entry(h, _list, list) {
+ if (!strcmp(h->name, exp_info->name)) {
+ mutex_unlock(_list_lock);
+ pr_err("dma_heap: Already registered heap named %s\n",
+ exp_info->name);
+ err_ret = ERR_PTR(-EINVAL);
+ goto err3;
+ }
+ }
+
+ /* Add heap to the list */
list_add(>list, _list);
mutex_unlock(_list_lock);
return heap;
+err3:
+ device_destroy(dma_heap_class, heap->heap_devt);
err2:
cdev_del(>heap_cdev);
err1:
--
2.25.1
[PATCH v3] dma-buf: fix racing conflict of dma_heap_add()
Racing conflict could be:
task A task B
list_for_each_entry
strcmp(h->name))
list_for_each_entry
strcmp(h->name)
kzallockzalloc
.. .
device_create device_create
list_add
list_add
The root cause is that task B has no idea about the fact someone
else(A) has inserted heap with same name when it calls list_add,
so a potential collision occurs.
v1:
https://lore.kernel.org/all/tycp286mb2323950197f60fc3473123b7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2: Narrow down locking scope, check the existence of heap before
insertion, as suggested by Andrew Davis.
v2->v3: Remove double checking.
Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7
Signed-off-by: Dawei Li
---
drivers/dma-buf/dma-heap.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..7a25e98259ea 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -233,18 +233,6 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
return ERR_PTR(-EINVAL);
}
- /* check the name is unique */
- mutex_lock(_list_lock);
- list_for_each_entry(h, _list, list) {
- if (!strcmp(h->name, exp_info->name)) {
- mutex_unlock(_list_lock);
- pr_err("dma_heap: Already registered heap named %s\n",
- exp_info->name);
- return ERR_PTR(-EINVAL);
- }
- }
- mutex_unlock(_list_lock);
-
heap = kzalloc(sizeof(*heap), GFP_KERNEL);
if (!heap)
return ERR_PTR(-ENOMEM);
@@ -283,13 +271,26 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
err_ret = ERR_CAST(dev_ret);
goto err2;
}
- /* Add heap to the list */
+
mutex_lock(_list_lock);
+ /* check the name is unique */
+ list_for_each_entry(h, _list, list) {
+ if (!strcmp(h->name, exp_info->name)) {
+ mutex_unlock(_list_lock);
+ pr_err("dma_heap: Already registered heap named %s\n",
+ exp_info->name);
+ err_ret = ERR_PTR(-EINVAL);
+ goto err3;
+ }
+ }
+
+ /* Add heap to the list */
list_add(>list, _list);
mutex_unlock(_list_lock);
return heap;
-
+err3:
+ device_destroy(dma_heap_class, heap->heap_devt);
err2:
cdev_del(>heap_cdev);
err1:
--
2.25.1
[PATCH v2] dma-buf: fix racing conflict of dma_heap_add()
Racing conflict could be:
task A task B
list_for_each_entry
strcmp(h->name))
list_for_each_entry
strcmp(h->name)
kzallockzalloc
.. .
device_create device_create
list_add
list_add
The root cause is that task B has no idea about the fact someone
else(A) has inserted heap with same name when it calls list_add,
so a potential collision occurs.
v1:
https://lore.kernel.org/all/tycp286mb2323950197f60fc3473123b7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2: Narrow down locking scope, check the existence of heap before
insertion, as suggested by Andrew Davis.
Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7
Signed-off-by: Dawei Li
---
drivers/dma-buf/dma-heap.c | 37 -
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..1c787a147e3a 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -216,9 +216,21 @@ const char *dma_heap_get_name(struct dma_heap *heap)
return heap->name;
}
+static inline bool dma_heap_exist(const char *name)
+{
+ struct dma_heap *h;
+
+ list_for_each_entry(h, _list, list) {
+ if (!strcmp(h->name, name))
+ return true;
+ }
+
+ return false;
+}
+
struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
{
- struct dma_heap *heap, *h, *err_ret;
+ struct dma_heap *heap, *err_ret;
struct device *dev_ret;
unsigned int minor;
int ret;
@@ -235,13 +247,11 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
/* check the name is unique */
mutex_lock(_list_lock);
- list_for_each_entry(h, _list, list) {
- if (!strcmp(h->name, exp_info->name)) {
- mutex_unlock(_list_lock);
- pr_err("dma_heap: Already registered heap named %s\n",
- exp_info->name);
- return ERR_PTR(-EINVAL);
- }
+ if (dma_heap_exist(exp_info->name)) {
+ mutex_unlock(_list_lock);
+ pr_err("dma_heap: Already registered heap named %s\n",
+ exp_info->name);
+ return ERR_PTR(-EINVAL);
}
mutex_unlock(_list_lock);
@@ -283,13 +293,22 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
err_ret = ERR_CAST(dev_ret);
goto err2;
}
+
/* Add heap to the list */
mutex_lock(_list_lock);
+ if (unlikely(dma_heap_exist(exp_info->name))) {
+ mutex_unlock(_list_lock);
+ pr_err("dma_heap: Already registered heap named %s\n",
+ exp_info->name);
+ err_ret = ERR_PTR(-EINVAL);
+ goto err3;
+ }
list_add(>list, _list);
mutex_unlock(_list_lock);
return heap;
-
+err3:
+ device_destroy(dma_heap_class, heap->heap_devt);
err2:
cdev_del(>heap_cdev);
err1:
--
2.25.1
[PATCH] dma-buf: fix racing conflict of dma_heap_add()
Racing conflict could be:
task A task B
list_for_each_entry
strcmp(h->name))
list_for_each_entry
strcmp(h->name)
kzallockzalloc
.. .
device_create device_create
list_add
list_add
The root cause is that task B has no idea about the fact someone
else(A) has inserted heap with same name when it calls list_add,
so a potential collision occurs.
Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7
Signed-off-by: Dawei Li
---
drivers/dma-buf/dma-heap.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..ff44c2777b04 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -243,11 +243,12 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
return ERR_PTR(-EINVAL);
}
}
- mutex_unlock(_list_lock);
heap = kzalloc(sizeof(*heap), GFP_KERNEL);
- if (!heap)
+ if (!heap) {
+ mutex_unlock(_list_lock);
return ERR_PTR(-ENOMEM);
+ }
heap->name = exp_info->name;
heap->ops = exp_info->ops;
@@ -284,7 +285,6 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
goto err2;
}
/* Add heap to the list */
- mutex_lock(_list_lock);
list_add(>list, _list);
mutex_unlock(_list_lock);
@@ -296,6 +296,7 @@ struct dma_heap *dma_heap_add(const struct
dma_heap_export_info *exp_info)
xa_erase(_heap_minors, minor);
err0:
kfree(heap);
+ mutex_unlock(_list_lock);
return err_ret;
}
--
2.25.1
[PATCH] drm/vmwgfx: Protect pin_user_pages with mmap_lock
This patch includes changes below:
1) pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling mmap_read_lock() & mmap_read_unlock().
2) fix & refactor the incorrect exception handling procedure in
vmw_mksstat_add_ioctl().
based-on branch: vmwgfx/drm-misc-fixes
based commit: d8a79c03054911c375a2252627a429c9bc4615b6
Signed-off-by: Dawei Li
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 2aceac7856e2..ec40a3364e0a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1020,9 +1020,9 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
const size_t num_pages_info = PFN_UP(arg->info_len);
const size_t num_pages_strs = PFN_UP(arg->strs_len);
long desc_len;
- long nr_pinned_stat;
- long nr_pinned_info;
- long nr_pinned_strs;
+ long nr_pinned_stat = 0;
+ long nr_pinned_info = 0;
+ long nr_pinned_strs = 0;
struct page *pages_stat[ARRAY_SIZE(pdesc->statPPNs)];
struct page *pages_info[ARRAY_SIZE(pdesc->infoPPNs)];
struct page *pages_strs[ARRAY_SIZE(pdesc->strsPPNs)];
@@ -1076,6 +1076,7 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
if (desc_len < 0) {
atomic_set(_priv->mksstat_user_pids[slot], 0);
+ __free_page(page);
return -EFAULT;
}
@@ -1083,28 +1084,33 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
reset_ppn_array(pdesc->infoPPNs, ARRAY_SIZE(pdesc->infoPPNs));
reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
+ /* pin_user_pages() needs protection of mmap_lock */
+ mmap_read_lock(current->mm);
+
/* Pin mksGuestStat user pages and store those in the instance
descriptor */
nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat, NULL);
if (num_pages_stat != nr_pinned_stat)
- goto err_pin_stat;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_stat; ++i)
pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
nr_pinned_info = pin_user_pages(arg->info, num_pages_info,
FOLL_LONGTERM, pages_info, NULL);
if (num_pages_info != nr_pinned_info)
- goto err_pin_info;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_info; ++i)
pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs,
FOLL_LONGTERM, pages_strs, NULL);
if (num_pages_strs != nr_pinned_strs)
- goto err_pin_strs;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_strs; ++i)
pdesc->strsPPNs[i] = page_to_pfn(pages_strs[i]);
+ mmap_read_unlock(current->mm);
+
/* Send the descriptor to the host via a hypervisor call. The
mksGuestStat
pages will remain in use until the user requests a matching remove
stats
or a stats reset occurs. */
@@ -1119,15 +1125,15 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
return 0;
-err_pin_strs:
+__err_pin_pages:
+ mmap_read_unlock(current->mm);
+
if (nr_pinned_strs > 0)
unpin_user_pages(pages_strs, nr_pinned_strs);
-err_pin_info:
if (nr_pinned_info > 0)
unpin_user_pages(pages_info, nr_pinned_info);
-err_pin_stat:
if (nr_pinned_stat > 0)
unpin_user_pages(pages_stat, nr_pinned_stat);
--
2.25.1
