Hi,

On Wed, Feb 12, 2014 at 09:52:17AM +0200, Cristian Stoica wrote:
> If 'open /dev/crypto' fails, all allocated resources must be freed
> before "open" returns; "close" can't be called to clean-up since
> there is no file descriptor after a failed "open".
> 
> Signed-off-by: Cristian Stoica <cristian.sto...@freescale.com>
> ---
>  ioctl.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/ioctl.c b/ioctl.c
> index 4838870..d4e83f4 100644
> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -440,7 +440,7 @@ static void cryptask_routine(struct work_struct *work)
>  static int
>  cryptodev_open(struct inode *inode, struct file *filp)
>  {
> -     struct todo_list_item *tmp;
> +     struct todo_list_item *tmp, *tmp_next;
>       struct crypt_priv *pcr;
>       int i;
>  
> @@ -466,7 +466,7 @@ cryptodev_open(struct inode *inode, struct file *filp)
>       for (i = 0; i < DEF_COP_RINGSIZE; i++) {
>               tmp = kzalloc(sizeof(struct todo_list_item), GFP_KERNEL);
>               if (!tmp)
> -                     return -ENOMEM;
> +                     goto err_ringalloc;

This would mean that in case pcr allocation fails, the code would try to
destroy mutexes which haven't been initialised at all. Probably I was a
bit unclear here: just leave the code as is, returning -ENOMEM at this
point is perfectly fine.

>               pcr->itemcount++;
>               ddebug(2, "allocated new item at %p", tmp);
>               list_add(&tmp->__hook, &pcr->free.list);
> @@ -475,6 +475,20 @@ cryptodev_open(struct inode *inode, struct file *filp)
>       ddebug(2, "Cryptodev handle initialised, %d elements in queue",
>                       DEF_COP_RINGSIZE);
>       return 0;
> +
> +/* In case of errors, free any memory allocated so far */
> +err_ringalloc:
> +     list_for_each_entry_safe(tmp, tmp_next, &pcr->free.list, __hook) {
> +             list_del(&tmp->__hook);
> +             kfree(tmp);
> +     }
> +     mutex_destroy(&pcr->done.lock);
> +     mutex_destroy(&pcr->todo.lock);
> +     mutex_destroy(&pcr->free.lock);
> +     mutex_destroy(&pcr->fcrypt.sem);
> +     kfree(pcr);
> +     filp->private_data = NULL;
> +     return -ENOMEM;
>  }
>  
>  static int
> -- 
> 1.8.3.1
> 
> 

Best wishes, Phil

_______________________________________________
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Reply via email to