linux-kernel - Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

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]

Message-ID: <1484331062.2527.22.camel@HansenPartnership.com>
Date:   Fri, 13 Jan 2017 10:11:02 -0800
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Jason Gunthorpe <jgunthorpe@...idianresearch.com>
Cc:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        open list <linux-kernel@...r.kernel.org>,
        linux-security-module@...r.kernel.org,
        tpmdd-devel@...ts.sourceforge.net
Subject: Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager
 via a device link /dev/tpms<n>

On Fri, 2017-01-13 at 11:01 -0700, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2017 at 09:40:08AM -0800, James Bottomley wrote:
> > On Fri, 2017-01-13 at 10:25 -0700, Jason Gunthorpe wrote:
> > > On Thu, Jan 12, 2017 at 10:56:28PM +0200, Jarkko Sakkinen wrote:
> > > 
> > > > >  dev_t tpm_devt;
> > > > 
> > > > But they should have different major device numbers.
> > > 
> > > major/minors don't really matter these days since they are
> > > dynamic
> > 
> > Right, although we have this weird piece of code:
> > 
> > 
> > 	if (chip->dev_num == 0)
> > 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > 	else
> > 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
> > 
> > So the first TPM device gets the MISC_MAJOR with TPM_MINOR and the
> > rest
> > get the dynamic major/minor.  It means when you do an ls on a
> > complex
> > system you get something like:
> 
> The TPM has always used a static major/minor for tpm0 and dynamic for
> the following. Originally this used a misc_device and did:
> 
>         } else if (chip->dev_num == 0)
>                 chip->vendor.miscdev.minor = TPM_MINOR;
>         else
>                 chip->vendor.miscdev.minor = MISC_DYNAMIC_MINOR;
> 
> When we moved to struct device the !0 tpms got a dynamic major as
> well.
> 
> I don't really know what the broad kernel feeling is on historical
> major/minor stability - this was mainly continuing what had always
> been done in this code for pre-udev userspace compatibility.
> 
> Does it harm anything?

Devices are either supposed to be static, so you can rely on the
major/minor without needing udev or fully dynamic, so you need udev to
get the nodes and you may not rely on the major/minor number.  The
potential harm is that we're neither one nor the other, which sounds
like an accident waiting to happen if someone decides to rely on our
numbers but gets the wrong node.  Chances are that, since most linux
devices are fully dynamic, no-one ever thinks of relying on fixed
major/minor numbers anymore, so everyone avoids this banana skin and
perhaps it doesn't matter.

James