mm/vmalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: "Hailong.Liu" <hailong.liu@oppo.com>
vm_map_ram check return value of vb_alloc by IS_ERR. if
vm_map_ram(page, 0, 0) , vb_alloc(0, GFP_KERNEL) would return NULL
which cause kernel panic by vmap_pages_range_noflush。fix this by
return ERR_PTR(-EINVAL) if size is 0.
Signed-off-by: Hailong.Liu <hailong.liu@oppo.com>
---
Changes since v1 [1]:
- Return ERR_PTR(-EINVAL) or not check IS_ERR_OR_NULL
BTW, Barry suggests me that if count is 0, return directly, in my
opinion, change return value is more resonable.
[1] https://lore.kernel.org/all/84d7cd03-1cf8-401a-8edf-2524db0bd6d5@oppo.com/
mm/vmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index a3fedb3ee0db..c430a999805b 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2173,7 +2173,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask)
* get_order(0) returns funny result. Just warn and terminate
* early.
*/
- return NULL;
+ return ERR_PTR(-EINVAL);
}
order = get_order(size);
--
2.34.1
On Fri, Apr 19, 2024 at 06:16:43PM +0800, hailong.liu@oppo.com wrote: > From: "Hailong.Liu" <hailong.liu@oppo.com> > > vm_map_ram check return value of vb_alloc by IS_ERR. if > vm_map_ram(page, 0, 0) , vb_alloc(0, GFP_KERNEL) would return NULL > which cause kernel panic by vmap_pages_range_noflush。fix this by > return ERR_PTR(-EINVAL) if size is 0. > > Signed-off-by: Hailong.Liu <hailong.liu@oppo.com> > --- > Changes since v1 [1]: > - Return ERR_PTR(-EINVAL) or not check IS_ERR_OR_NULL > > BTW, Barry suggests me that if count is 0, return directly, in my > opinion, change return value is more resonable. > > [1] https://lore.kernel.org/all/84d7cd03-1cf8-401a-8edf-2524db0bd6d5@oppo.com/ > > mm/vmalloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index a3fedb3ee0db..c430a999805b 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2173,7 +2173,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask) > * get_order(0) returns funny result. Just warn and terminate > * early. > */ > - return NULL; > + return ERR_PTR(-EINVAL); > } > order = get_order(size); > > -- > 2.34.1 > Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com> -- Uladzislau Rezki
On Fri, Apr 19, 2024 at 6:17 PM <hailong.liu@oppo.com> wrote: > > From: "Hailong.Liu" <hailong.liu@oppo.com> > > vm_map_ram check return value of vb_alloc by IS_ERR. if > vm_map_ram(page, 0, 0) , vb_alloc(0, GFP_KERNEL) would return NULL > which cause kernel panic by vmap_pages_range_noflush。fix this by > return ERR_PTR(-EINVAL) if size is 0. > > Signed-off-by: Hailong.Liu <hailong.liu@oppo.com> > --- > Changes since v1 [1]: > - Return ERR_PTR(-EINVAL) or not check IS_ERR_OR_NULL > > BTW, Barry suggests me that if count is 0, return directly, in my > opinion, change return value is more resonable. > > [1] https://lore.kernel.org/all/84d7cd03-1cf8-401a-8edf-2524db0bd6d5@oppo.com/ > > mm/vmalloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index a3fedb3ee0db..c430a999805b 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2173,7 +2173,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask) > * get_order(0) returns funny result. Just warn and terminate > * early. > */ > - return NULL; > + return ERR_PTR(-EINVAL); might be ZERO_SIZE_PTR. > } > order = get_order(size); > > -- > 2.34.1
On Mon, 22. Apr 11:46, Barry Song wrote: > On Fri, Apr 19, 2024 at 6:17 PM <hailong.liu@oppo.com> wrote: > > > > From: "Hailong.Liu" <hailong.liu@oppo.com> > > > > vm_map_ram check return value of vb_alloc by IS_ERR. if > > vm_map_ram(page, 0, 0) , vb_alloc(0, GFP_KERNEL) would return NULL > > which cause kernel panic by vmap_pages_range_noflush。fix this by > > return ERR_PTR(-EINVAL) if size is 0. > > > > Signed-off-by: Hailong.Liu <hailong.liu@oppo.com> > > --- > > Changes since v1 [1]: > > - Return ERR_PTR(-EINVAL) or not check IS_ERR_OR_NULL > > > > BTW, Barry suggests me that if count is 0, return directly, in my > > opinion, change return value is more resonable. > > > > [1] https://lore.kernel.org/all/84d7cd03-1cf8-401a-8edf-2524db0bd6d5@oppo.com/ > > > > mm/vmalloc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > > index a3fedb3ee0db..c430a999805b 100644 > > --- a/mm/vmalloc.c > > +++ b/mm/vmalloc.c > > @@ -2173,7 +2173,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask) > > * get_order(0) returns funny result. Just warn and terminate > > * early. > > */ > > - return NULL; > > + return ERR_PTR(-EINVAL); > > might be ZERO_SIZE_PTR. Hi Barry, Hi Barry, I use ERR_PTR(-EINVAL) to keep consistency with the return value of the function alloc_vmap_area. if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align))) return ERR_PTR(-EINVAL); IMO, ZERO_SIZE_PTR is used by slab and can not be catched by IS_ERR(). -- Best Regards, Hailong.
On Mon, Apr 22, 2024 at 6:39 PM Hailong Liu <hailong.liu@oppo.com> wrote: > > On Mon, 22. Apr 11:46, Barry Song wrote: > > On Fri, Apr 19, 2024 at 6:17 PM <hailong.liu@oppo.com> wrote: > > > > > > From: "Hailong.Liu" <hailong.liu@oppo.com> > > > > > > vm_map_ram check return value of vb_alloc by IS_ERR. if > > > vm_map_ram(page, 0, 0) , vb_alloc(0, GFP_KERNEL) would return NULL > > > which cause kernel panic by vmap_pages_range_noflush。fix this by > > > return ERR_PTR(-EINVAL) if size is 0. > > > > > > Signed-off-by: Hailong.Liu <hailong.liu@oppo.com> > > > --- > > > Changes since v1 [1]: > > > - Return ERR_PTR(-EINVAL) or not check IS_ERR_OR_NULL > > > > > > BTW, Barry suggests me that if count is 0, return directly, in my > > > opinion, change return value is more resonable. > > > > > > [1] https://lore.kernel.org/all/84d7cd03-1cf8-401a-8edf-2524db0bd6d5@oppo.com/ > > > > > > mm/vmalloc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > > > index a3fedb3ee0db..c430a999805b 100644 > > > --- a/mm/vmalloc.c > > > +++ b/mm/vmalloc.c > > > @@ -2173,7 +2173,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask) > > > * get_order(0) returns funny result. Just warn and terminate > > > * early. > > > */ > > > - return NULL; > > > + return ERR_PTR(-EINVAL); > > > > might be ZERO_SIZE_PTR. > > Hi Barry, > Hi Barry, I use ERR_PTR(-EINVAL) to keep consistency with the return > value of the function alloc_vmap_area. > > if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align))) > return ERR_PTR(-EINVAL); > > IMO, ZERO_SIZE_PTR is used by slab and can not be catched by IS_ERR(). Ok. it seems not worth to change two places. Reviewed-by: Barry Song <baohua@kernel.org> > > -- > Best Regards, > Hailong.
© 2016 - 2024 Red Hat, Inc.