staging: ion: create one device entry per heap

Message ID 1505746726-11443-1-git-send-email-benjamin.gaignard@linaro.org
State Superseded
Headers show
Series
  • staging: ion: create one device entry per heap
Related show

Commit Message

Benjamin Gaignard Sept. 18, 2017, 2:58 p.m.
Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.

Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

---
 drivers/staging/android/ion/ion-ioctl.c |  9 +++++++--
 drivers/staging/android/ion/ion.c       | 20 ++++++++++++++------
 drivers/staging/android/ion/ion.h       | 10 +++++++---
 3 files changed, 28 insertions(+), 11 deletions(-)

-- 
2.7.4

Comments

Dan Carpenter Sept. 19, 2017, 9:40 a.m. | #1
On Mon, Sep 18, 2017 at 04:58:46PM +0200, Benjamin Gaignard wrote:
> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

> +static int validate_ioctl_arg(struct file *filp,

> +			      unsigned int cmd, union ion_ioctl_arg *arg)

>  {

>  	int ret = 0;

> +	int mask = 1 << iminor(filp->f_inode);

>  

>  	switch (cmd) {

>  	case ION_IOC_HEAP_QUERY:

> @@ -35,6 +37,9 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>  		ret |= arg->query.reserved1 != 0;

>  		ret |= arg->query.reserved2 != 0;

>  		break;

> +	case ION_IOC_ALLOC:

> +		ret = !(arg->allocation.heap_id_mask & mask);



validate_ioctl_arg() is really convoluted.  From reading just the patch
I at first thought we were returning 1 on failure.  Just say:

	if (!(arg->allocation.heap_id_mask & mask))
		return -EINVAL;
	return 0;

If you want to fix the surrounding code in a separate patch that would
be good.  It would be more clear to say:

		if (arg->query.reserved0 != 0 ||
		    arg->query.reserved1 != 0 ||
		    arg->query.reserved2 != 0)
			return -EINVAL;

> +		break;

>  	default:

>  		break;

>  	}

> @@ -70,7 +75,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

>  	if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))

>  		return -EFAULT;

>  

> -	ret = validate_ioctl_arg(cmd, &data);

> +	ret = validate_ioctl_arg(filp, cmd, &data);

>  	if (WARN_ON_ONCE(ret))

>  		return ret;

>  

> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c

> index 93e2c90..5144f1a 100644

> --- a/drivers/staging/android/ion/ion.c

> +++ b/drivers/staging/android/ion/ion.c

> @@ -40,6 +40,8 @@

>  

>  #include "ion.h"

>  

> +#define ION_DEV_MAX 32

> +

>  static struct ion_device *internal_dev;

>  static int heap_id;

>  

> @@ -541,11 +543,21 @@ void ion_device_add_heap(struct ion_heap *heap)

>  {

>  	struct dentry *debug_file;

>  	struct ion_device *dev = internal_dev;

> +	int ret;

>  

>  	if (!heap->ops->allocate || !heap->ops->free)

>  		pr_err("%s: can not add heap with invalid ops struct.\n",

>  		       __func__);

>  


I don't think it can happen in current code but we should proabably have
a check here for:

	if (heap_id >= ION_DEV_MAX)
		return -EBUSY;

(It's possible I have missed something).


> +	heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);

> +	dev_set_name(&heap->ddev, "ion%d", heap_id);

> +	device_initialize(&heap->ddev);

> +	cdev_init(&heap->chrdev, &ion_fops);

> +	heap->chrdev.owner = THIS_MODULE;

> +	ret = cdev_device_add(&heap->chrdev, &heap->ddev);

> +	if (ret < 0)

> +		return;

> +

>  	spin_lock_init(&heap->free_lock);

>  	heap->free_list_size = 0;

>  


regards,
dan carpenter
Benjamin Gaignard Sept. 19, 2017, 10:07 a.m. | #2
2017-09-19 11:40 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Mon, Sep 18, 2017 at 04:58:46PM +0200, Benjamin Gaignard wrote:

>> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>> +static int validate_ioctl_arg(struct file *filp,

>> +                           unsigned int cmd, union ion_ioctl_arg *arg)

>>  {

>>       int ret = 0;

>> +     int mask = 1 << iminor(filp->f_inode);

>>

>>       switch (cmd) {

>>       case ION_IOC_HEAP_QUERY:

>> @@ -35,6 +37,9 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>>               ret |= arg->query.reserved1 != 0;

>>               ret |= arg->query.reserved2 != 0;

>>               break;

>> +     case ION_IOC_ALLOC:

>> +             ret = !(arg->allocation.heap_id_mask & mask);

>

>

> validate_ioctl_arg() is really convoluted.  From reading just the patch

> I at first thought we were returning 1 on failure.  Just say:

>

>         if (!(arg->allocation.heap_id_mask & mask))

>                 return -EINVAL;

>         return 0;

>

> If you want to fix the surrounding code in a separate patch that would

> be good.  It would be more clear to say:

>

>                 if (arg->query.reserved0 != 0 ||

>                     arg->query.reserved1 != 0 ||

>                     arg->query.reserved2 != 0)

>                         return -EINVAL;


I agree I will add a fix for that in next version

>

>> +             break;

>>       default:

>>               break;

>>       }

>> @@ -70,7 +75,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

>>       if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))

>>               return -EFAULT;

>>

>> -     ret = validate_ioctl_arg(cmd, &data);

>> +     ret = validate_ioctl_arg(filp, cmd, &data);

>>       if (WARN_ON_ONCE(ret))

>>               return ret;

>>

>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c

>> index 93e2c90..5144f1a 100644

>> --- a/drivers/staging/android/ion/ion.c

>> +++ b/drivers/staging/android/ion/ion.c

>> @@ -40,6 +40,8 @@

>>

>>  #include "ion.h"

>>

>> +#define ION_DEV_MAX 32

>> +

>>  static struct ion_device *internal_dev;

>>  static int heap_id;

>>

>> @@ -541,11 +543,21 @@ void ion_device_add_heap(struct ion_heap *heap)

>>  {

>>       struct dentry *debug_file;

>>       struct ion_device *dev = internal_dev;

>> +     int ret;

>>

>>       if (!heap->ops->allocate || !heap->ops->free)

>>               pr_err("%s: can not add heap with invalid ops struct.\n",

>>                      __func__);

>>

>

> I don't think it can happen in current code but we should proabably have

> a check here for:

>

>         if (heap_id >= ION_DEV_MAX)

>                 return -EBUSY;

>

> (It's possible I have missed something).

>


You are right I will add that

Thanks
>

>> +     heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);

>> +     dev_set_name(&heap->ddev, "ion%d", heap_id);

>> +     device_initialize(&heap->ddev);

>> +     cdev_init(&heap->chrdev, &ion_fops);

>> +     heap->chrdev.owner = THIS_MODULE;

>> +     ret = cdev_device_add(&heap->chrdev, &heap->ddev);

>> +     if (ret < 0)

>> +             return;

>> +

>>       spin_lock_init(&heap->free_lock);

>>       heap->free_list_size = 0;

>>

>

> regards,

> dan carpenter

>

>




-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
Tomas Winkler Sept. 19, 2017, 10:15 a.m. | #3
On Tue, Sep 19, 2017 at 1:07 PM, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> 2017-09-19 11:40 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:

>> On Mon, Sep 18, 2017 at 04:58:46PM +0200, Benjamin Gaignard wrote:

>>> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>>> +static int validate_ioctl_arg(struct file *filp,

>>> +                           unsigned int cmd, union ion_ioctl_arg *arg)

>>>  {

>>>       int ret = 0;

>>> +     int mask = 1 << iminor(filp->f_inode);

>>>

>>>       switch (cmd) {

>>>       case ION_IOC_HEAP_QUERY:

>>> @@ -35,6 +37,9 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>>>               ret |= arg->query.reserved1 != 0;

>>>               ret |= arg->query.reserved2 != 0;

>>>               break;

>>> +     case ION_IOC_ALLOC:

>>> +             ret = !(arg->allocation.heap_id_mask & mask);

>>

>>

>> validate_ioctl_arg() is really convoluted.  From reading just the patch

>> I at first thought we were returning 1 on failure.  Just say:

>>

>>         if (!(arg->allocation.heap_id_mask & mask))

>>                 return -EINVAL;

>>         return 0;

>>

>> If you want to fix the surrounding code in a separate patch that would

>> be good.  It would be more clear to say:

>>

>>                 if (arg->query.reserved0 != 0 ||

>>                     arg->query.reserved1 != 0 ||

>>                     arg->query.reserved2 != 0)

>>                         return -EINVAL;

>

> I agree I will add a fix for that in next version

>

>>

>>> +             break;

>>>       default:

>>>               break;

>>>       }

>>> @@ -70,7 +75,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

>>>       if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))

>>>               return -EFAULT;

>>>

>>> -     ret = validate_ioctl_arg(cmd, &data);

>>> +     ret = validate_ioctl_arg(filp, cmd, &data);

>>>       if (WARN_ON_ONCE(ret))

>>>               return ret;

>>>

>>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c

>>> index 93e2c90..5144f1a 100644

>>> --- a/drivers/staging/android/ion/ion.c

>>> +++ b/drivers/staging/android/ion/ion.c

>>> @@ -40,6 +40,8 @@

>>>

>>>  #include "ion.h"

>>>

>>> +#define ION_DEV_MAX 32

>>> +

>>>  static struct ion_device *internal_dev;

>>>  static int heap_id;

>>>

>>> @@ -541,11 +543,21 @@ void ion_device_add_heap(struct ion_heap *heap)

>>>  {

>>>       struct dentry *debug_file;

>>>       struct ion_device *dev = internal_dev;

>>> +     int ret;

>>>

>>>       if (!heap->ops->allocate || !heap->ops->free)

>>>               pr_err("%s: can not add heap with invalid ops struct.\n",

>>>                      __func__);

>>>

>>

>> I don't think it can happen in current code but we should proabably have

>> a check here for:

>>

>>         if (heap_id >= ION_DEV_MAX)

>>                 return -EBUSY;

>>

>> (It's possible I have missed something).

>>

>

> You are right I will add that

>

> Thanks

>>

>>> +     heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);

>>> +     dev_set_name(&heap->ddev, "ion%d", heap_id);

>>> +     device_initialize(&heap->ddev);

>>> +     cdev_init(&heap->chrdev, &ion_fops);

>>> +     heap->chrdev.owner = THIS_MODULE;

>>> +     ret = cdev_device_add(&heap->chrdev, &heap->ddev);

>>> +     if (ret < 0)

>>> +             return;

>>> +

>>>       spin_lock_init(&heap->free_lock);

>>>       heap->free_list_size = 0;


What will happen to an application which looks for /dev/ion?

Thanks
Tomas
Benjamin Gaignard Sept. 19, 2017, 10:20 a.m. | #4
2017-09-19 12:15 GMT+02:00 Tomas Winkler <tomasw@gmail.com>:
> On Tue, Sep 19, 2017 at 1:07 PM, Benjamin Gaignard

> <benjamin.gaignard@linaro.org> wrote:

>> 2017-09-19 11:40 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:

>>> On Mon, Sep 18, 2017 at 04:58:46PM +0200, Benjamin Gaignard wrote:

>>>> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>>>> +static int validate_ioctl_arg(struct file *filp,

>>>> +                           unsigned int cmd, union ion_ioctl_arg *arg)

>>>>  {

>>>>       int ret = 0;

>>>> +     int mask = 1 << iminor(filp->f_inode);

>>>>

>>>>       switch (cmd) {

>>>>       case ION_IOC_HEAP_QUERY:

>>>> @@ -35,6 +37,9 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

>>>>               ret |= arg->query.reserved1 != 0;

>>>>               ret |= arg->query.reserved2 != 0;

>>>>               break;

>>>> +     case ION_IOC_ALLOC:

>>>> +             ret = !(arg->allocation.heap_id_mask & mask);

>>>

>>>

>>> validate_ioctl_arg() is really convoluted.  From reading just the patch

>>> I at first thought we were returning 1 on failure.  Just say:

>>>

>>>         if (!(arg->allocation.heap_id_mask & mask))

>>>                 return -EINVAL;

>>>         return 0;

>>>

>>> If you want to fix the surrounding code in a separate patch that would

>>> be good.  It would be more clear to say:

>>>

>>>                 if (arg->query.reserved0 != 0 ||

>>>                     arg->query.reserved1 != 0 ||

>>>                     arg->query.reserved2 != 0)

>>>                         return -EINVAL;

>>

>> I agree I will add a fix for that in next version

>>

>>>

>>>> +             break;

>>>>       default:

>>>>               break;

>>>>       }

>>>> @@ -70,7 +75,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

>>>>       if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))

>>>>               return -EFAULT;

>>>>

>>>> -     ret = validate_ioctl_arg(cmd, &data);

>>>> +     ret = validate_ioctl_arg(filp, cmd, &data);

>>>>       if (WARN_ON_ONCE(ret))

>>>>               return ret;

>>>>

>>>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c

>>>> index 93e2c90..5144f1a 100644

>>>> --- a/drivers/staging/android/ion/ion.c

>>>> +++ b/drivers/staging/android/ion/ion.c

>>>> @@ -40,6 +40,8 @@

>>>>

>>>>  #include "ion.h"

>>>>

>>>> +#define ION_DEV_MAX 32

>>>> +

>>>>  static struct ion_device *internal_dev;

>>>>  static int heap_id;

>>>>

>>>> @@ -541,11 +543,21 @@ void ion_device_add_heap(struct ion_heap *heap)

>>>>  {

>>>>       struct dentry *debug_file;

>>>>       struct ion_device *dev = internal_dev;

>>>> +     int ret;

>>>>

>>>>       if (!heap->ops->allocate || !heap->ops->free)

>>>>               pr_err("%s: can not add heap with invalid ops struct.\n",

>>>>                      __func__);

>>>>

>>>

>>> I don't think it can happen in current code but we should proabably have

>>> a check here for:

>>>

>>>         if (heap_id >= ION_DEV_MAX)

>>>                 return -EBUSY;

>>>

>>> (It's possible I have missed something).

>>>

>>

>> You are right I will add that

>>

>> Thanks

>>>

>>>> +     heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);

>>>> +     dev_set_name(&heap->ddev, "ion%d", heap_id);

>>>> +     device_initialize(&heap->ddev);

>>>> +     cdev_init(&heap->chrdev, &ion_fops);

>>>> +     heap->chrdev.owner = THIS_MODULE;

>>>> +     ret = cdev_device_add(&heap->chrdev, &heap->ddev);

>>>> +     if (ret < 0)

>>>> +             return;

>>>> +

>>>>       spin_lock_init(&heap->free_lock);

>>>>       heap->free_list_size = 0;

>

> What will happen to an application which looks for /dev/ion?


/dev/ion will no more exist with this patch.
Since ion ABI have already change a lot I don't think that could
be a problem to change also ion device.

>

> Thanks

> Tomas
Greg Kroah-Hartman Sept. 19, 2017, 10:59 a.m. | #5
On Tue, Sep 19, 2017 at 12:20:15PM +0200, Benjamin Gaignard wrote:
> 2017-09-19 12:15 GMT+02:00 Tomas Winkler <tomasw@gmail.com>:

> > On Tue, Sep 19, 2017 at 1:07 PM, Benjamin Gaignard

> > <benjamin.gaignard@linaro.org> wrote:

> >> 2017-09-19 11:40 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:

> >>> On Mon, Sep 18, 2017 at 04:58:46PM +0200, Benjamin Gaignard wrote:

> >>>> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

> >>>> +static int validate_ioctl_arg(struct file *filp,

> >>>> +                           unsigned int cmd, union ion_ioctl_arg *arg)

> >>>>  {

> >>>>       int ret = 0;

> >>>> +     int mask = 1 << iminor(filp->f_inode);

> >>>>

> >>>>       switch (cmd) {

> >>>>       case ION_IOC_HEAP_QUERY:

> >>>> @@ -35,6 +37,9 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)

> >>>>               ret |= arg->query.reserved1 != 0;

> >>>>               ret |= arg->query.reserved2 != 0;

> >>>>               break;

> >>>> +     case ION_IOC_ALLOC:

> >>>> +             ret = !(arg->allocation.heap_id_mask & mask);

> >>>

> >>>

> >>> validate_ioctl_arg() is really convoluted.  From reading just the patch

> >>> I at first thought we were returning 1 on failure.  Just say:

> >>>

> >>>         if (!(arg->allocation.heap_id_mask & mask))

> >>>                 return -EINVAL;

> >>>         return 0;

> >>>

> >>> If you want to fix the surrounding code in a separate patch that would

> >>> be good.  It would be more clear to say:

> >>>

> >>>                 if (arg->query.reserved0 != 0 ||

> >>>                     arg->query.reserved1 != 0 ||

> >>>                     arg->query.reserved2 != 0)

> >>>                         return -EINVAL;

> >>

> >> I agree I will add a fix for that in next version

> >>

> >>>

> >>>> +             break;

> >>>>       default:

> >>>>               break;

> >>>>       }

> >>>> @@ -70,7 +75,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

> >>>>       if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))

> >>>>               return -EFAULT;

> >>>>

> >>>> -     ret = validate_ioctl_arg(cmd, &data);

> >>>> +     ret = validate_ioctl_arg(filp, cmd, &data);

> >>>>       if (WARN_ON_ONCE(ret))

> >>>>               return ret;

> >>>>

> >>>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c

> >>>> index 93e2c90..5144f1a 100644

> >>>> --- a/drivers/staging/android/ion/ion.c

> >>>> +++ b/drivers/staging/android/ion/ion.c

> >>>> @@ -40,6 +40,8 @@

> >>>>

> >>>>  #include "ion.h"

> >>>>

> >>>> +#define ION_DEV_MAX 32

> >>>> +

> >>>>  static struct ion_device *internal_dev;

> >>>>  static int heap_id;

> >>>>

> >>>> @@ -541,11 +543,21 @@ void ion_device_add_heap(struct ion_heap *heap)

> >>>>  {

> >>>>       struct dentry *debug_file;

> >>>>       struct ion_device *dev = internal_dev;

> >>>> +     int ret;

> >>>>

> >>>>       if (!heap->ops->allocate || !heap->ops->free)

> >>>>               pr_err("%s: can not add heap with invalid ops struct.\n",

> >>>>                      __func__);

> >>>>

> >>>

> >>> I don't think it can happen in current code but we should proabably have

> >>> a check here for:

> >>>

> >>>         if (heap_id >= ION_DEV_MAX)

> >>>                 return -EBUSY;

> >>>

> >>> (It's possible I have missed something).

> >>>

> >>

> >> You are right I will add that

> >>

> >> Thanks

> >>>

> >>>> +     heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);

> >>>> +     dev_set_name(&heap->ddev, "ion%d", heap_id);

> >>>> +     device_initialize(&heap->ddev);

> >>>> +     cdev_init(&heap->chrdev, &ion_fops);

> >>>> +     heap->chrdev.owner = THIS_MODULE;

> >>>> +     ret = cdev_device_add(&heap->chrdev, &heap->ddev);

> >>>> +     if (ret < 0)

> >>>> +             return;

> >>>> +

> >>>>       spin_lock_init(&heap->free_lock);

> >>>>       heap->free_list_size = 0;

> >

> > What will happen to an application which looks for /dev/ion?

> 

> /dev/ion will no more exist with this patch.

> Since ion ABI have already change a lot I don't think that could

> be a problem to change also ion device.


So, what did you just break in userspace?  :(

Do you also have userspace patches submitted anywhere to handle this
change?

thanks,

greg k-h

Patch

diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
index d9f8b14..ff06ce3 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -25,9 +25,11 @@  union ion_ioctl_arg {
 	struct ion_heap_query query;
 };
 
-static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
+static int validate_ioctl_arg(struct file *filp,
+			      unsigned int cmd, union ion_ioctl_arg *arg)
 {
 	int ret = 0;
+	int mask = 1 << iminor(filp->f_inode);
 
 	switch (cmd) {
 	case ION_IOC_HEAP_QUERY:
@@ -35,6 +37,9 @@  static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
 		ret |= arg->query.reserved1 != 0;
 		ret |= arg->query.reserved2 != 0;
 		break;
+	case ION_IOC_ALLOC:
+		ret = !(arg->allocation.heap_id_mask & mask);
+		break;
 	default:
 		break;
 	}
@@ -70,7 +75,7 @@  long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
 		return -EFAULT;
 
-	ret = validate_ioctl_arg(cmd, &data);
+	ret = validate_ioctl_arg(filp, cmd, &data);
 	if (WARN_ON_ONCE(ret))
 		return ret;
 
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 93e2c90..5144f1a 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,8 @@ 
 
 #include "ion.h"
 
+#define ION_DEV_MAX 32
+
 static struct ion_device *internal_dev;
 static int heap_id;
 
@@ -541,11 +543,21 @@  void ion_device_add_heap(struct ion_heap *heap)
 {
 	struct dentry *debug_file;
 	struct ion_device *dev = internal_dev;
+	int ret;
 
 	if (!heap->ops->allocate || !heap->ops->free)
 		pr_err("%s: can not add heap with invalid ops struct.\n",
 		       __func__);
 
+	heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);
+	dev_set_name(&heap->ddev, "ion%d", heap_id);
+	device_initialize(&heap->ddev);
+	cdev_init(&heap->chrdev, &ion_fops);
+	heap->chrdev.owner = THIS_MODULE;
+	ret = cdev_device_add(&heap->chrdev, &heap->ddev);
+	if (ret < 0)
+		return;
+
 	spin_lock_init(&heap->free_lock);
 	heap->free_list_size = 0;
 
@@ -595,13 +607,9 @@  static int ion_device_create(void)
 	if (!idev)
 		return -ENOMEM;
 
-	idev->dev.minor = MISC_DYNAMIC_MINOR;
-	idev->dev.name = "ion";
-	idev->dev.fops = &ion_fops;
-	idev->dev.parent = NULL;
-	ret = misc_register(&idev->dev);
+	ret = alloc_chrdev_region(&idev->devt, 0, ION_DEV_MAX, "ion");
 	if (ret) {
-		pr_err("ion: failed to register misc device.\n");
+		pr_err("ion: unable to allocate major\n");
 		kfree(idev);
 		return ret;
 	}
diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index 621e5f7..ef51ff5 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -17,6 +17,7 @@ 
 #ifndef _ION_H
 #define _ION_H
 
+#include <linux/cdev.h>
 #include <linux/device.h>
 #include <linux/dma-direction.h>
 #include <linux/kref.h>
@@ -26,7 +27,6 @@ 
 #include <linux/sched.h>
 #include <linux/shrinker.h>
 #include <linux/types.h>
-#include <linux/miscdevice.h>
 
 #include "../uapi/ion.h"
 
@@ -90,13 +90,13 @@  void ion_buffer_destroy(struct ion_buffer *buffer);
 
 /**
  * struct ion_device - the metadata of the ion device node
- * @dev:		the actual misc device
+ * @dev:		the actual device
  * @buffers:		an rb tree of all the existing buffers
  * @buffer_lock:	lock protecting the tree of buffers
  * @lock:		rwsem protecting the tree of heaps and clients
  */
 struct ion_device {
-	struct miscdevice dev;
+	dev_t devt;
 	struct rb_root buffers;
 	struct mutex buffer_lock;
 	struct rw_semaphore lock;
@@ -152,6 +152,8 @@  struct ion_heap_ops {
  * struct ion_heap - represents a heap in the system
  * @node:		rb node to put the heap on the device's tree of heaps
  * @dev:		back pointer to the ion_device
+ * @ddev:		device structure
+ * @chrdev:		associated character device
  * @type:		type of heap
  * @ops:		ops struct as above
  * @flags:		flags
@@ -176,6 +178,8 @@  struct ion_heap_ops {
 struct ion_heap {
 	struct plist_node node;
 	struct ion_device *dev;
+	struct device ddev;
+	struct cdev chrdev;
 	enum ion_heap_type type;
 	struct ion_heap_ops *ops;
 	unsigned long flags;