Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1286923

Re: [PATCH 2/2] keys, trusted: seal with a policy

Show all headers | View raw

On Tue, 2015-12-08 at 22:24 +0200, Jarkko Sakkinen wrote:
> On Tue, Dec 08, 2015 at 01:01:02PM +0200, Jarkko Sakkinen wrote:
> > On Tue, Dec 08, 2015 at 09:35:05AM +1100, James Morris wrote:
> > > On Mon, 7 Dec 2015, Jarkko Sakkinen wrote:
> > > 
> > > > On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote:
> > > > > On Wed, 18 Nov 2015, Jarkko Sakkinen wrote:
> > > > > 
> > > > > > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote:
> > > > > > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote:
> > > > > > > 
> > > > > > > >  			}
> > > > > > > >  			break;
> > > > > > > > +		case Opt_policydigest:
> > > > > > > > +			if (!tpm2 ||
> > > > > > > > +			    strlen(args[0].from) != (2 * opt->digest_len))
> > > > > > > > +				return -EINVAL;
> > > > > > > > +			kfree(opt->policydigest);
> > > > > > > > +			opt->policydigest = kzalloc(opt->digest_len,
> > > > > > > > +						    GFP_KERNEL);

You're allocating the exact amount of storage needed.  There's no reason
to use kzalloc here or elsewhere in the patch.

> > > > > > > 
> > > > > > > Is it correct to kfree opt->policydigest here before allocating it?
> > > > > > 
> > > > > > I think so. The same option might be encountered multiple times.
> > > > > 
> > > > > This would surely signify an error?
> > > > 
> > > > I'm following the semantics of other options. That's why I implemented
> > > > it that way for example:
> > > > 
> > > > keyctl add trusted kmk "new 32 keyhandle=0x80000000 keyhandle=0x80000000"
> > > > 
> > > > is perfectly OK. I just thought that it'd be more odd if this option
> > > > behaved in a different way...
> > > 
> > > It seems broken to me -- if you're messing up keyctl commands you might 
> > > want to know about it, but we should remain consistent.
> > 
> > So should I return error if policyhandle/digest appears a second time? I
> > agree that it'd be better to return -EINVAL.
> > 
> > The existing behavior is such that any option can appear multiple times
> > and I chose to be consistent with that.
> 
> Mimi, David?

I don't have a problem with changing the existing behavior to allow the
options to be specified only once.

BTW, you might want to fail the getoptions() parsing earlier, rather
than waiting until after the while loop to test "policydigest_len !=
opt->digest_len".   In both Opt_hash and Opt_policydigest you can check
to see if the other option has already been specified.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH 2/2] keys, trusted: seal with a policy Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-07 10:20 +0100
  Re: [PATCH 2/2] keys, trusted: seal with a policy James Morris <jmorris@namei.org> - 2015-12-07 23:40 +0100
    Re: [PATCH 2/2] keys, trusted: seal with a policy Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-08 12:10 +0100
      Re: [PATCH 2/2] keys, trusted: seal with a policy Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-08 21:30 +0100
        Re: [PATCH 2/2] keys, trusted: seal with a policy Mimi Zohar <zohar@linux.vnet.ibm.com> - 2015-12-09 01:00 +0100
          Re: [PATCH 2/2] keys, trusted: seal with a policy Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-09 15:30 +0100
            Re: [PATCH 2/2] keys, trusted: seal with a policy Mimi Zohar <zohar@linux.vnet.ibm.com> - 2015-12-09 17:20 +0100

csiph-web