* [PATCH 0/3] drm/vmwgfx: Fine-tuning for 13 function implementations
@ 2016-09-23 16:34 SF Markus Elfring
2016-09-23 16:36 ` [PATCH 1/3] drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl() SF Markus Elfring
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: SF Markus Elfring @ 2016-09-23 16:34 UTC (permalink / raw)
To: dri-devel, linux-graphics-maintainer, David Airlie, Sinclair Yeh,
Thomas Hellstrom
Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 23 Sep 2016 18:24:32 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (3):
Use kmalloc_array() in vmw_surface_define_ioctl()
Use memdup_user() rather than duplicating its implementation
Adjust checks for null pointers in 13 functions
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 56 ++++++++++++++-------------------
1 file changed, 23 insertions(+), 33 deletions(-)
--
2.10.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/3] drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl() 2016-09-23 16:34 [PATCH 0/3] drm/vmwgfx: Fine-tuning for 13 function implementations SF Markus Elfring @ 2016-09-23 16:36 ` SF Markus Elfring 2016-09-23 16:38 ` [PATCH 2/3] drm/vmwgfx: Use memdup_user() rather than duplicating its implementation SF Markus Elfring 2016-09-23 16:39 ` [PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions SF Markus Elfring 2 siblings, 0 replies; 5+ messages in thread From: SF Markus Elfring @ 2016-09-23 16:36 UTC (permalink / raw) To: dri-devel, linux-graphics-maintainer, David Airlie, Sinclair Yeh, Thomas Hellstrom Cc: Julia Lawall, kernel-janitors, LKML From: Markus Elfring <elfring@users.sourceforge.net> Date: Thu, 22 Sep 2016 21:54:33 +0200 Multiplications for the size determination of memory allocations indicated that array data structures should be processed. Thus use the corresponding function "kmalloc_array". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index c2a721a..f557549 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -763,14 +763,16 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, memcpy(srf->mip_levels, req->mip_levels, sizeof(srf->mip_levels)); srf->num_sizes = num_sizes; user_srf->size = size; - - srf->sizes = kmalloc(srf->num_sizes * sizeof(*srf->sizes), GFP_KERNEL); + srf->sizes = kmalloc_array(srf->num_sizes, + sizeof(*srf->sizes), + GFP_KERNEL); if (unlikely(srf->sizes == NULL)) { ret = -ENOMEM; goto out_no_sizes; } - srf->offsets = kmalloc(srf->num_sizes * sizeof(*srf->offsets), - GFP_KERNEL); + srf->offsets = kmalloc_array(srf->num_sizes, + sizeof(*srf->offsets), + GFP_KERNEL); if (unlikely(srf->offsets == NULL)) { ret = -ENOMEM; goto out_no_offsets; -- 2.10.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] drm/vmwgfx: Use memdup_user() rather than duplicating its implementation 2016-09-23 16:34 [PATCH 0/3] drm/vmwgfx: Fine-tuning for 13 function implementations SF Markus Elfring 2016-09-23 16:36 ` [PATCH 1/3] drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl() SF Markus Elfring @ 2016-09-23 16:38 ` SF Markus Elfring 2016-09-23 16:39 ` [PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions SF Markus Elfring 2 siblings, 0 replies; 5+ messages in thread From: SF Markus Elfring @ 2016-09-23 16:38 UTC (permalink / raw) To: dri-devel, linux-graphics-maintainer, David Airlie, Sinclair Yeh, Thomas Hellstrom Cc: LKML, kernel-janitors, Julia Lawall From: Markus Elfring <elfring@users.sourceforge.net> Date: Fri, 23 Sep 2016 17:26:02 +0200 * Reuse existing functionality from memdup_user() instead of keeping duplicate source code. * Try this copy operation before allocating memory for the data structure member "offsets". * Delete the local variable "user_sizes" which became unnecessary with this refactoring. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index f557549..15504c6 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -700,7 +700,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, struct drm_vmw_surface_create_req *req = &arg->req; struct drm_vmw_surface_arg *rep = &arg->rep; struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile; - struct drm_vmw_size __user *user_sizes; int ret; int i, j; uint32_t cur_bo_offset; @@ -763,11 +762,11 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, memcpy(srf->mip_levels, req->mip_levels, sizeof(srf->mip_levels)); srf->num_sizes = num_sizes; user_srf->size = size; - srf->sizes = kmalloc_array(srf->num_sizes, - sizeof(*srf->sizes), - GFP_KERNEL); - if (unlikely(srf->sizes == NULL)) { - ret = -ENOMEM; + srf->sizes = memdup_user((struct drm_vmw_size __user *)(unsigned long) + req->size_addr, + sizeof(*srf->sizes) * srf->num_sizes); + if (IS_ERR(srf->sizes)) { + ret = PTR_ERR(srf->sizes); goto out_no_sizes; } srf->offsets = kmalloc_array(srf->num_sizes, @@ -778,16 +777,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, goto out_no_offsets; } - user_sizes = (struct drm_vmw_size __user *)(unsigned long) - req->size_addr; - - ret = copy_from_user(srf->sizes, user_sizes, - srf->num_sizes * sizeof(*srf->sizes)); - if (unlikely(ret != 0)) { - ret = -EFAULT; - goto out_no_copy; - } - srf->base_size = *srf->sizes; srf->autogen_filter = SVGA3D_TEX_FILTER_NONE; srf->multisample_count = 0; -- 2.10.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions 2016-09-23 16:34 [PATCH 0/3] drm/vmwgfx: Fine-tuning for 13 function implementations SF Markus Elfring 2016-09-23 16:36 ` [PATCH 1/3] drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl() SF Markus Elfring 2016-09-23 16:38 ` [PATCH 2/3] drm/vmwgfx: Use memdup_user() rather than duplicating its implementation SF Markus Elfring @ 2016-09-23 16:39 ` SF Markus Elfring 2016-09-26 22:20 ` Sinclair Yeh 2 siblings, 1 reply; 5+ messages in thread From: SF Markus Elfring @ 2016-09-23 16:39 UTC (permalink / raw) To: dri-devel, linux-graphics-maintainer, David Airlie, Sinclair Yeh, Thomas Hellstrom Cc: LKML, kernel-janitors, Julia Lawall From: Markus Elfring <elfring@users.sourceforge.net> Date: Fri, 23 Sep 2016 17:53:49 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script "checkpatch.pl" can point information out like the following. Comparison to NULL could be written !… Thus fix the affected source code places. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 15504c6..b445ce9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -324,7 +324,7 @@ static void vmw_hw_surface_destroy(struct vmw_resource *res) if (res->id != -1) { cmd = vmw_fifo_reserve(dev_priv, vmw_surface_destroy_size()); - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "destruction.\n"); return; @@ -397,7 +397,7 @@ static int vmw_legacy_srf_create(struct vmw_resource *res) submit_size = vmw_surface_define_size(srf); cmd = vmw_fifo_reserve(dev_priv, submit_size); - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "creation.\n"); ret = -ENOMEM; @@ -446,11 +446,10 @@ static int vmw_legacy_srf_dma(struct vmw_resource *res, uint8_t *cmd; struct vmw_private *dev_priv = res->dev_priv; - BUG_ON(val_buf->bo == NULL); - + BUG_ON(!val_buf->bo); submit_size = vmw_surface_dma_size(srf); cmd = vmw_fifo_reserve(dev_priv, submit_size); - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "DMA.\n"); return -ENOMEM; @@ -538,7 +537,7 @@ static int vmw_legacy_srf_destroy(struct vmw_resource *res) submit_size = vmw_surface_destroy_size(); cmd = vmw_fifo_reserve(dev_priv, submit_size); - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "eviction.\n"); return -ENOMEM; @@ -578,7 +577,7 @@ static int vmw_surface_init(struct vmw_private *dev_priv, int ret; struct vmw_resource *res = &srf->res; - BUG_ON(res_free == NULL); + BUG_ON(!res_free); if (!dev_priv->has_mob) vmw_fifo_resource_inc(dev_priv); ret = vmw_resource_init(dev_priv, res, true, res_free, @@ -747,7 +746,7 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, } user_srf = kzalloc(sizeof(*user_srf), GFP_KERNEL); - if (unlikely(user_srf == NULL)) { + if (unlikely(!user_srf)) { ret = -ENOMEM; goto out_no_user_srf; } @@ -772,7 +771,7 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, srf->offsets = kmalloc_array(srf->num_sizes, sizeof(*srf->offsets), GFP_KERNEL); - if (unlikely(srf->offsets == NULL)) { + if (unlikely(!srf->offsets)) { ret = -ENOMEM; goto out_no_offsets; } @@ -914,7 +913,7 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv, ret = -EINVAL; base = ttm_base_object_lookup_for_ref(dev_priv->tdev, handle); - if (unlikely(base == NULL)) { + if (unlikely(!base)) { DRM_ERROR("Could not find surface to reference.\n"); goto out_no_lookup; } @@ -1060,7 +1059,7 @@ static int vmw_gb_surface_create(struct vmw_resource *res) cmd = vmw_fifo_reserve(dev_priv, submit_len); cmd2 = (typeof(cmd2))cmd; - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "creation.\n"); ret = -ENOMEM; @@ -1126,7 +1125,7 @@ static int vmw_gb_surface_bind(struct vmw_resource *res, submit_size = sizeof(*cmd1) + (res->backup_dirty ? sizeof(*cmd2) : 0); cmd1 = vmw_fifo_reserve(dev_priv, submit_size); - if (unlikely(cmd1 == NULL)) { + if (unlikely(!cmd1)) { DRM_ERROR("Failed reserving FIFO space for surface " "binding.\n"); return -ENOMEM; @@ -1176,7 +1175,7 @@ static int vmw_gb_surface_unbind(struct vmw_resource *res, submit_size = sizeof(*cmd3) + (readback ? sizeof(*cmd1) : sizeof(*cmd2)); cmd = vmw_fifo_reserve(dev_priv, submit_size); - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "unbinding.\n"); return -ENOMEM; @@ -1235,7 +1234,7 @@ static int vmw_gb_surface_destroy(struct vmw_resource *res) vmw_binding_res_list_scrub(&res->binding_head); cmd = vmw_fifo_reserve(dev_priv, sizeof(*cmd)); - if (unlikely(cmd == NULL)) { + if (unlikely(!cmd)) { DRM_ERROR("Failed reserving FIFO space for surface " "destruction.\n"); mutex_unlock(&dev_priv->binding_mutex); @@ -1401,7 +1400,7 @@ int vmw_gb_surface_reference_ioctl(struct drm_device *dev, void *data, user_srf = container_of(base, struct vmw_user_surface, prime.base); srf = &user_srf->srf; - if (srf->res.backup == NULL) { + if (!srf->res.backup) { DRM_ERROR("Shared GB surface is missing a backup buffer.\n"); goto out_bad_resource; } @@ -1515,7 +1514,7 @@ int vmw_surface_gb_priv_define(struct drm_device *dev, } user_srf = kzalloc(sizeof(*user_srf), GFP_KERNEL); - if (unlikely(user_srf == NULL)) { + if (unlikely(!user_srf)) { ret = -ENOMEM; goto out_no_user_srf; } -- 2.10.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions 2016-09-23 16:39 ` [PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions SF Markus Elfring @ 2016-09-26 22:20 ` Sinclair Yeh 0 siblings, 0 replies; 5+ messages in thread From: Sinclair Yeh @ 2016-09-26 22:20 UTC (permalink / raw) To: SF Markus Elfring Cc: dri-devel, linux-graphics-maintainer, David Airlie, Thomas Hellstrom, LKML, kernel-janitors, Julia Lawall This series looks good to me: Reviewed-by: Sinclair Yeh <syeh@vmware.com> Thanks! On Fri, Sep 23, 2016 at 06:39:04PM +0200, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Fri, 23 Sep 2016 17:53:49 +0200 > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > The script "checkpatch.pl" can point information out like the following. > > Comparison to NULL could be written !… > > Thus fix the affected source code places. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > index 15504c6..b445ce9 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > @@ -324,7 +324,7 @@ static void vmw_hw_surface_destroy(struct vmw_resource *res) > if (res->id != -1) { > > cmd = vmw_fifo_reserve(dev_priv, vmw_surface_destroy_size()); > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "destruction.\n"); > return; > @@ -397,7 +397,7 @@ static int vmw_legacy_srf_create(struct vmw_resource *res) > > submit_size = vmw_surface_define_size(srf); > cmd = vmw_fifo_reserve(dev_priv, submit_size); > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "creation.\n"); > ret = -ENOMEM; > @@ -446,11 +446,10 @@ static int vmw_legacy_srf_dma(struct vmw_resource *res, > uint8_t *cmd; > struct vmw_private *dev_priv = res->dev_priv; > > - BUG_ON(val_buf->bo == NULL); > - > + BUG_ON(!val_buf->bo); > submit_size = vmw_surface_dma_size(srf); > cmd = vmw_fifo_reserve(dev_priv, submit_size); > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "DMA.\n"); > return -ENOMEM; > @@ -538,7 +537,7 @@ static int vmw_legacy_srf_destroy(struct vmw_resource *res) > > submit_size = vmw_surface_destroy_size(); > cmd = vmw_fifo_reserve(dev_priv, submit_size); > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "eviction.\n"); > return -ENOMEM; > @@ -578,7 +577,7 @@ static int vmw_surface_init(struct vmw_private *dev_priv, > int ret; > struct vmw_resource *res = &srf->res; > > - BUG_ON(res_free == NULL); > + BUG_ON(!res_free); > if (!dev_priv->has_mob) > vmw_fifo_resource_inc(dev_priv); > ret = vmw_resource_init(dev_priv, res, true, res_free, > @@ -747,7 +746,7 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, > } > > user_srf = kzalloc(sizeof(*user_srf), GFP_KERNEL); > - if (unlikely(user_srf == NULL)) { > + if (unlikely(!user_srf)) { > ret = -ENOMEM; > goto out_no_user_srf; > } > @@ -772,7 +771,7 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, > srf->offsets = kmalloc_array(srf->num_sizes, > sizeof(*srf->offsets), > GFP_KERNEL); > - if (unlikely(srf->offsets == NULL)) { > + if (unlikely(!srf->offsets)) { > ret = -ENOMEM; > goto out_no_offsets; > } > @@ -914,7 +913,7 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv, > > ret = -EINVAL; > base = ttm_base_object_lookup_for_ref(dev_priv->tdev, handle); > - if (unlikely(base == NULL)) { > + if (unlikely(!base)) { > DRM_ERROR("Could not find surface to reference.\n"); > goto out_no_lookup; > } > @@ -1060,7 +1059,7 @@ static int vmw_gb_surface_create(struct vmw_resource *res) > > cmd = vmw_fifo_reserve(dev_priv, submit_len); > cmd2 = (typeof(cmd2))cmd; > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "creation.\n"); > ret = -ENOMEM; > @@ -1126,7 +1125,7 @@ static int vmw_gb_surface_bind(struct vmw_resource *res, > submit_size = sizeof(*cmd1) + (res->backup_dirty ? sizeof(*cmd2) : 0); > > cmd1 = vmw_fifo_reserve(dev_priv, submit_size); > - if (unlikely(cmd1 == NULL)) { > + if (unlikely(!cmd1)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "binding.\n"); > return -ENOMEM; > @@ -1176,7 +1175,7 @@ static int vmw_gb_surface_unbind(struct vmw_resource *res, > > submit_size = sizeof(*cmd3) + (readback ? sizeof(*cmd1) : sizeof(*cmd2)); > cmd = vmw_fifo_reserve(dev_priv, submit_size); > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "unbinding.\n"); > return -ENOMEM; > @@ -1235,7 +1234,7 @@ static int vmw_gb_surface_destroy(struct vmw_resource *res) > vmw_binding_res_list_scrub(&res->binding_head); > > cmd = vmw_fifo_reserve(dev_priv, sizeof(*cmd)); > - if (unlikely(cmd == NULL)) { > + if (unlikely(!cmd)) { > DRM_ERROR("Failed reserving FIFO space for surface " > "destruction.\n"); > mutex_unlock(&dev_priv->binding_mutex); > @@ -1401,7 +1400,7 @@ int vmw_gb_surface_reference_ioctl(struct drm_device *dev, void *data, > > user_srf = container_of(base, struct vmw_user_surface, prime.base); > srf = &user_srf->srf; > - if (srf->res.backup == NULL) { > + if (!srf->res.backup) { > DRM_ERROR("Shared GB surface is missing a backup buffer.\n"); > goto out_bad_resource; > } > @@ -1515,7 +1514,7 @@ int vmw_surface_gb_priv_define(struct drm_device *dev, > } > > user_srf = kzalloc(sizeof(*user_srf), GFP_KERNEL); > - if (unlikely(user_srf == NULL)) { > + if (unlikely(!user_srf)) { > ret = -ENOMEM; > goto out_no_user_srf; > } > -- > 2.10.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-26 22:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-23 16:34 [PATCH 0/3] drm/vmwgfx: Fine-tuning for 13 function implementations SF Markus Elfring 2016-09-23 16:36 ` [PATCH 1/3] drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl() SF Markus Elfring 2016-09-23 16:38 ` [PATCH 2/3] drm/vmwgfx: Use memdup_user() rather than duplicating its implementation SF Markus Elfring 2016-09-23 16:39 ` [PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions SF Markus Elfring 2016-09-26 22:20 ` Sinclair Yeh
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).