Re: Patch "media: coda: fix memory corruption in case more than 32 instances are opened" has been added to the 4.4-stable tree

[Greg KH]

On Fri, Dec 06, 2019 at 04:24:44PM -0500, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
> 
>     media: coda: fix memory corruption in case more than 32 instances are opened
> 
> to the 4.4-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      media-coda-fix-memory-corruption-in-case-more-than-3.patch
> and it can be found in the queue-4.4 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
> 
> 
> 
> commit 0aecc2dd80345b10cd6ab210a77b3ecc83ca4bdc
> Author: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> Date:   Tue Nov 6 05:40:54 2018 -0500
> 
>     media: coda: fix memory corruption in case more than 32 instances are opened
>     
>     [ Upstream commit 649cfc2bdfeeb98ff7d8fdff0af3f8fb9c8da50f ]
>     
>     The ffz() return value is undefined if the instance mask does not
>     contain any zeros. If it returned 32, the following set_bit would
>     corrupt the debugfs_root pointer.
>     Switch to IDA for context index allocation. This also removes the
>     artificial 32 instance limit for all except CodaDx6.
>     
>     Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
>     Signed-off-by: Hans Verkuil <hansverk@xxxxxxxxx>
>     Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx>
>     Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> 
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index 323aad3c89de6..154aa7d73a8d2 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -17,6 +17,7 @@
>  #include <linux/firmware.h>
>  #include <linux/gcd.h>
>  #include <linux/genalloc.h>
> +#include <linux/idr.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/irq.h>
> @@ -1644,17 +1645,6 @@ int coda_decoder_queue_init(void *priv, struct vb2_queue *src_vq,
>  	return coda_queue_init(priv, dst_vq);
>  }
>  
> -static int coda_next_free_instance(struct coda_dev *dev)
> -{
> -	int idx = ffz(dev->instance_mask);
> -
> -	if ((idx < 0) ||
> -	    (dev->devtype->product == CODA_DX6 && idx > CODADX6_MAX_INSTANCES))
> -		return -EBUSY;
> -
> -	return idx;
> -}
> -
>  /*
>   * File operations
>   */
> @@ -1663,7 +1653,8 @@ static int coda_open(struct file *file)
>  {
>  	struct video_device *vdev = video_devdata(file);
>  	struct coda_dev *dev = video_get_drvdata(vdev);
> -	struct coda_ctx *ctx = NULL;
> +	struct coda_ctx *ctx;
> +	unsigned int max = ~0;
>  	char *name;
>  	int ret;
>  	int idx;
> @@ -1672,12 +1663,13 @@ static int coda_open(struct file *file)
>  	if (!ctx)
>  		return -ENOMEM;
>  
> -	idx = coda_next_free_instance(dev);
> +	if (dev->devtype->product == CODA_DX6)
> +		max = CODADX6_MAX_INSTANCES - 1;
> +	idx = ida_alloc_max(&dev->ida, max, GFP_KERNEL);
>  	if (idx < 0) {
>  		ret = idx;
>  		goto err_coda_max;
>  	}
> -	set_bit(idx, &dev->instance_mask);
>  
>  	name = kasprintf(GFP_KERNEL, "context%d", idx);
>  	if (!name) {
> @@ -1771,8 +1763,8 @@ err_clk_per:
>  err_pm_get:
>  	v4l2_fh_del(&ctx->fh);
>  	v4l2_fh_exit(&ctx->fh);
> -	clear_bit(ctx->idx, &dev->instance_mask);
>  err_coda_name_init:
> +	ida_free(&dev->ida, ctx->idx);
>  err_coda_max:
>  	kfree(ctx);
>  	return ret;
> @@ -1811,7 +1803,7 @@ static int coda_release(struct file *file)
>  	pm_runtime_put_sync(&dev->plat_dev->dev);
>  	v4l2_fh_del(&ctx->fh);
>  	v4l2_fh_exit(&ctx->fh);
> -	clear_bit(ctx->idx, &dev->instance_mask);
> +	ida_free(&dev->ida, ctx->idx);
>  	if (ctx->ops->release)
>  		ctx->ops->release(ctx);
>  	debugfs_remove_recursive(ctx->debugfs_entry);
> @@ -2192,6 +2184,7 @@ static int coda_probe(struct platform_device *pdev)
>  
>  	mutex_init(&dev->dev_mutex);
>  	mutex_init(&dev->coda_mutex);
> +	ida_init(&dev->ida);
>  
>  	dev->debugfs_root = debugfs_create_dir("coda", NULL);
>  	if (!dev->debugfs_root)
> @@ -2276,6 +2269,7 @@ static int coda_remove(struct platform_device *pdev)
>  	coda_free_aux_buf(dev, &dev->tempbuf);
>  	coda_free_aux_buf(dev, &dev->workbuf);
>  	debugfs_remove_recursive(dev->debugfs_root);
> +	ida_destroy(&dev->ida);
>  	return 0;
>  }
>  
> diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
> index 96532b06bd9e1..239f6bb2fca42 100644
> --- a/drivers/media/platform/coda/coda.h
> +++ b/drivers/media/platform/coda/coda.h
> @@ -16,6 +16,7 @@
>  #define __CODA_H__
>  
>  #include <linux/debugfs.h>
> +#include <linux/idr.h>
>  #include <linux/irqreturn.h>
>  #include <linux/mutex.h>
>  #include <linux/kfifo.h>
> @@ -93,7 +94,7 @@ struct coda_dev {
>  	struct v4l2_m2m_dev	*m2m_dev;
>  	struct vb2_alloc_ctx	*alloc_ctx;
>  	struct list_head	instances;
> -	unsigned long		instance_mask;
> +	struct ida		ida;
>  	struct dentry		*debugfs_root;
>  };
>  

This breaks the build in 4.4, 4.9, and 4.14 kernels, so I've dropped it
from there.