archive mirror
 help / color / mirror / Atom feed
From: <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
To: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
Subject: Re: [PATCH 1/3] tpm-chip: Move idr_replace calls to appropriate places
Date: Thu, 27 Jul 2017 14:07:49 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20170709212430.GB19327-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/>

> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 67ec9d3..a353b7a 100644
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -327,11 +327,6 @@ static int tpm_add_char_device(struct tpm_chip
> *chip)
> >  		}
> >  	}
> >
> > -	/* Make the chip available. */
> > -	mutex_lock(&idr_lock);
> > -	idr_replace(&dev_nums_idr, chip, chip->dev_num);
> > -	mutex_unlock(&idr_lock);
> This is actually in the wrong place already, it needs to be done before
> cdev_device_add - this is because cdev_device_add generates the uevent to
> userspace which could trigger userspace to use the kernel device. So a patch
> to move it to the start of this function woud be good. The function would be
> better described as 'make visible'

I have looked again at the code and I am not sure this is an issue. The call to idr_replace is only necessary to enable in-kernel usage (i.e. RNG, IMA, ...) of the TPM, but it should not affect userspace in any way. So the location of the idr_replace call does not matter much, as long as the TPM is already initialized. In fact, the main purpose of this patch series (please see PATCH 3/3) is to export the device to userspace without calling idr_replace at all under some circumstances.

Or is there something I missed? The only function that ever tries to access the value stored by idr_replace is tpm_chip_find_get. It is usually called with TPM_ANY_NUM, selecting any TPM that might be present. If no TPM is present (or if idr_replace has not been called) the caller needs to deal with the situation already.

Check out the vibrant tech community on one of the world's most
engaging tech sites,!

      parent reply	other threads:[~2017-07-27 14:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-04 13:56 Alexander Steffen
     [not found] ` <20170704135648.7360-1-Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
2017-07-04 13:56   ` [PATCH 2/3] tpm-chip: Return TPM error codes from auto_startup functions Alexander Steffen
2017-07-04 13:56   ` [PATCH 3/3] tpm-chip: Export TPM device to user space even when startup failed Alexander Steffen
2017-07-09 21:24   ` [PATCH 1/3] tpm-chip: Move idr_replace calls to appropriate places Jason Gunthorpe
     [not found]     ` <20170709212430.GB19327-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/>
2017-07-27 14:07       ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \
    --to=alexander.steffen-d0qzbvysippwk0htik3j/ \
    --cc=jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/ \ \
    --subject='Re: [PATCH 1/3] tpm-chip: Move idr_replace calls to appropriate places' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).