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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
