linux-kernel - Re: [PATCH v2 2/2] staging: ion: create one device entry per heap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Sep 2017 14:39:05 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Benjamin Gaignard <benjamin.gaignard@...aro.org>
Cc:     driverdevel <devel@...verdev.osuosl.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Riley Andrews <riandrews@...roid.com>,
        Mark Brown <broonie@...nel.org>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH v2 2/2] staging: ion: create one device entry per heap

On Tue, Sep 19, 2017 at 01:55:36PM +0200, Benjamin Gaignard wrote:
> >> +
> >>       spin_lock_init(&heap->free_lock);
> >>       heap->free_list_size = 0;
> >>
> >> @@ -595,13 +610,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");
> >
> > Did you just change the major number for the device node as well?
> >
> My understanding of alloc_chrdev_region() is that major number is chosen
> dynamically but I don't understand the link with device node, sorry.

Yes, the major is chosen dynamically if you ask for it (like you are
here), but previously you were using the misc major number.  That might
break userspace really badly if it had hard-coded /dev (like lots of
android devices do...)

> > Wow, that's a lot of userspace breakage (both major number, and name),
> > how did you test this?
> 
> I had to write a test by myself:
> https://git.linaro.org/people/benjamin.gaignard/ion_test_application.git/log/?h=one_device_per_heap
> 
> Laura have tried to push a test VGEM but I believe it hasn't be
> accepted yet (I will check)

A "test" program is a bit different than "boots and runs a working
system", right?  Please work with the correct graphics people to ensure
this change works with their code, before resending it.

thanks,

greg k-h