* [PATCH] drm/rockchip: Don't allow zero sized gem buffer @ 2017-05-23 6:39 Jeffy Chen 2017-05-25 15:30 ` Sean Paul 0 siblings, 1 reply; 6+ messages in thread From: Jeffy Chen @ 2017-05-23 6:39 UTC (permalink / raw) To: linux-kernel Cc: seanpaul, tfiga, Jeffy Chen, Mark Yao, Heiko Stuebner, dri-devel, linux-rockchip, David Airlie, linux-arm-kernel The system would crash when trying to alloc zero sized gem buffer: [ 6.712435] Unable to handle kernel NULL pointer dereference at virtual address 00000010 <--ZERO_SIZE_PTR ... [ 6.757502] PC is at sg_alloc_table_from_pages+0x170/0x1ec Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index df9e570..8917922 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -315,6 +315,11 @@ struct rockchip_gem_object * struct drm_gem_object *obj; int ret; + if (!size) { + DRM_ERROR("gem buffer size is zero\n"); + return ERR_PTR(-EINVAL); + } + size = round_up(size, PAGE_SIZE); rk_obj = kzalloc(sizeof(*rk_obj), GFP_KERNEL); -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/rockchip: Don't allow zero sized gem buffer 2017-05-23 6:39 [PATCH] drm/rockchip: Don't allow zero sized gem buffer Jeffy Chen @ 2017-05-25 15:30 ` Sean Paul 2017-05-26 2:30 ` jeffy 0 siblings, 1 reply; 6+ messages in thread From: Sean Paul @ 2017-05-25 15:30 UTC (permalink / raw) To: Jeffy Chen Cc: linux-kernel, seanpaul, tfiga, Mark Yao, Heiko Stuebner, dri-devel, linux-rockchip, David Airlie, linux-arm-kernel On Tue, May 23, 2017 at 02:39:43PM +0800, Jeffy Chen wrote: > The system would crash when trying to alloc zero sized gem buffer: > [ 6.712435] Unable to handle kernel NULL pointer dereference at virtual address 00000010 <--ZERO_SIZE_PTR > ... > [ 6.757502] PC is at sg_alloc_table_from_pages+0x170/0x1ec It's unfortunate that you didn't include the entire stack trace. From code inspection, it seems like the 0 size comes from the fb_probe path? Is there somewhere in the helpers that you could check the mode is sane so all drivers can benefit? Sean > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > --- > > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index df9e570..8917922 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -315,6 +315,11 @@ struct rockchip_gem_object * > struct drm_gem_object *obj; > int ret; > > + if (!size) { > + DRM_ERROR("gem buffer size is zero\n"); > + return ERR_PTR(-EINVAL); > + } > + > size = round_up(size, PAGE_SIZE); > > rk_obj = kzalloc(sizeof(*rk_obj), GFP_KERNEL); > -- > 2.1.4 > -- Sean Paul, Software Engineer, Google / Chromium OS ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/rockchip: Don't allow zero sized gem buffer 2017-05-25 15:30 ` Sean Paul @ 2017-05-26 2:30 ` jeffy 2017-05-26 5:52 ` Christoph Hellwig 2017-05-26 13:49 ` Sean Paul 0 siblings, 2 replies; 6+ messages in thread From: jeffy @ 2017-05-26 2:30 UTC (permalink / raw) To: Sean Paul Cc: linux-kernel, tfiga, Mark Yao, Heiko Stuebner, dri-devel, linux-rockchip, David Airlie, linux-arm-kernel Hi sean, On 05/25/2017 11:30 PM, Sean Paul wrote: > On Tue, May 23, 2017 at 02:39:43PM +0800, Jeffy Chen wrote: >> The system would crash when trying to alloc zero sized gem buffer: >> [ 6.712435] Unable to handle kernel NULL pointer dereference at virtual address 00000010 <--ZERO_SIZE_PTR >> ... >> [ 6.757502] PC is at sg_alloc_table_from_pages+0x170/0x1ec > > It's unfortunate that you didn't include the entire stack trace. From code > inspection, it seems like the 0 size comes from the fb_probe path? Is there > somewhere in the helpers that you could check the mode is sane so all drivers > can benefit? hmm, sorry, i was testing it on chromeos 4.4 kernel, it turns out that we have a custom ioctl for userspace to create gem buffer(the same as exynos drm), which might get the the 0 size. but on upstream kernel, it could only be called by dump_create, and the drm_mode_create_dumb_ioctl already did the size check. will resent this patch, and rewrite the commit message, thanx. > > Sean > >> >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >> --- >> >> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c >> index df9e570..8917922 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c >> @@ -315,6 +315,11 @@ struct rockchip_gem_object * >> struct drm_gem_object *obj; >> int ret; >> >> + if (!size) { >> + DRM_ERROR("gem buffer size is zero\n"); >> + return ERR_PTR(-EINVAL); >> + } >> + >> size = round_up(size, PAGE_SIZE); >> >> rk_obj = kzalloc(sizeof(*rk_obj), GFP_KERNEL); >> -- >> 2.1.4 >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/rockchip: Don't allow zero sized gem buffer 2017-05-26 2:30 ` jeffy @ 2017-05-26 5:52 ` Christoph Hellwig 2017-05-26 6:50 ` Daniel Vetter 2017-05-26 13:49 ` Sean Paul 1 sibling, 1 reply; 6+ messages in thread From: Christoph Hellwig @ 2017-05-26 5:52 UTC (permalink / raw) To: jeffy Cc: Sean Paul, linux-kernel, tfiga, Mark Yao, Heiko Stuebner, dri-devel, linux-rockchip, David Airlie, linux-arm-kernel On Fri, May 26, 2017 at 10:30:09AM +0800, jeffy wrote: > Hi sean, > > On 05/25/2017 11:30 PM, Sean Paul wrote: > > On Tue, May 23, 2017 at 02:39:43PM +0800, Jeffy Chen wrote: > > > The system would crash when trying to alloc zero sized gem buffer: > > > [ 6.712435] Unable to handle kernel NULL pointer dereference at virtual address 00000010 <--ZERO_SIZE_PTR > > > ... > > > [ 6.757502] PC is at sg_alloc_table_from_pages+0x170/0x1ec > > > > It's unfortunate that you didn't include the entire stack trace. From code > > inspection, it seems like the 0 size comes from the fb_probe path? Is there > > somewhere in the helpers that you could check the mode is sane so all drivers > > can benefit? > > hmm, sorry, i was testing it on chromeos 4.4 kernel, it turns out that we > have a custom ioctl for userspace to create gem buffer(the same as exynos > drm), which might get the the 0 size. > > but on upstream kernel, it could only be called by dump_create, and the > drm_mode_create_dumb_ioctl already did the size check. > > will resent this patch, and rewrite the commit message, thanx. That suggests that this patch isn't needed at all. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/rockchip: Don't allow zero sized gem buffer 2017-05-26 5:52 ` Christoph Hellwig @ 2017-05-26 6:50 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2017-05-26 6:50 UTC (permalink / raw) To: Christoph Hellwig Cc: jeffy, Linux Kernel Mailing List, dri-devel, Tomasz Figa, open list:ARM/Rockchip SoC..., linux-arm-kernel On Fri, May 26, 2017 at 7:52 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Fri, May 26, 2017 at 10:30:09AM +0800, jeffy wrote: >> Hi sean, >> >> On 05/25/2017 11:30 PM, Sean Paul wrote: >> > On Tue, May 23, 2017 at 02:39:43PM +0800, Jeffy Chen wrote: >> > > The system would crash when trying to alloc zero sized gem buffer: >> > > [ 6.712435] Unable to handle kernel NULL pointer dereference at virtual address 00000010 <--ZERO_SIZE_PTR >> > > ... >> > > [ 6.757502] PC is at sg_alloc_table_from_pages+0x170/0x1ec >> > >> > It's unfortunate that you didn't include the entire stack trace. From code >> > inspection, it seems like the 0 size comes from the fb_probe path? Is there >> > somewhere in the helpers that you could check the mode is sane so all drivers >> > can benefit? >> >> hmm, sorry, i was testing it on chromeos 4.4 kernel, it turns out that we >> have a custom ioctl for userspace to create gem buffer(the same as exynos >> drm), which might get the the 0 size. >> >> but on upstream kernel, it could only be called by dump_create, and the >> drm_mode_create_dumb_ioctl already did the size check. >> >> will resent this patch, and rewrite the commit message, thanx. > > That suggests that this patch isn't needed at all. Yes, not needed for upstream. But next time around pls include the entire backtrace (or at least the relevant parts), not just the last line, so that we can figure this out directly. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/rockchip: Don't allow zero sized gem buffer 2017-05-26 2:30 ` jeffy 2017-05-26 5:52 ` Christoph Hellwig @ 2017-05-26 13:49 ` Sean Paul 1 sibling, 0 replies; 6+ messages in thread From: Sean Paul @ 2017-05-26 13:49 UTC (permalink / raw) To: jeffy Cc: Sean Paul, linux-kernel, tfiga, Mark Yao, Heiko Stuebner, dri-devel, linux-rockchip, David Airlie, linux-arm-kernel On Fri, May 26, 2017 at 10:30:09AM +0800, jeffy wrote: > Hi sean, > > On 05/25/2017 11:30 PM, Sean Paul wrote: > > On Tue, May 23, 2017 at 02:39:43PM +0800, Jeffy Chen wrote: > > > The system would crash when trying to alloc zero sized gem buffer: > > > [ 6.712435] Unable to handle kernel NULL pointer dereference at virtual address 00000010 <--ZERO_SIZE_PTR > > > ... > > > [ 6.757502] PC is at sg_alloc_table_from_pages+0x170/0x1ec > > > > It's unfortunate that you didn't include the entire stack trace. From code > > inspection, it seems like the 0 size comes from the fb_probe path? Is there > > somewhere in the helpers that you could check the mode is sane so all drivers > > can benefit? > > hmm, sorry, i was testing it on chromeos 4.4 kernel, it turns out that we > have a custom ioctl for userspace to create gem buffer(the same as exynos > drm), which might get the the 0 size. > > but on upstream kernel, it could only be called by dump_create, and the > drm_mode_create_dumb_ioctl already did the size check. Ah, ok. In that case, fix the custom ioctl such that it ensures we never call this function with size == 0, and upload it downstream with a CHROMIUM prefix. Sean > > will resent this patch, and rewrite the commit message, thanx. > > > > > Sean > > > > > > > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > > > --- > > > > > > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > > > index df9e570..8917922 100644 > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > > > @@ -315,6 +315,11 @@ struct rockchip_gem_object * > > > struct drm_gem_object *obj; > > > int ret; > > > > > > + if (!size) { > > > + DRM_ERROR("gem buffer size is zero\n"); > > > + return ERR_PTR(-EINVAL); > > > + } > > > + > > > size = round_up(size, PAGE_SIZE); > > > > > > rk_obj = kzalloc(sizeof(*rk_obj), GFP_KERNEL); > > > -- > > > 2.1.4 > > > > > > -- Sean Paul, Software Engineer, Google / Chromium OS ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-26 13:50 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-23 6:39 [PATCH] drm/rockchip: Don't allow zero sized gem buffer Jeffy Chen 2017-05-25 15:30 ` Sean Paul 2017-05-26 2:30 ` jeffy 2017-05-26 5:52 ` Christoph Hellwig 2017-05-26 6:50 ` Daniel Vetter 2017-05-26 13:49 ` Sean Paul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).