Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options

2015-12-14 Thread Jarkko Sakkinen
On Mon, Dec 14, 2015 at 08:46:33AM -0500, Mimi Zohar wrote:
> On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote:
> > The trusted keys option parsing allows specifying the same option
> > multiple times. The last option value specified is used.
> > 
> > This can be seen as a regression because:
> > 
> > * No gain.
> > * Could be problematic if there is be options dependent on other
> >   options.
> 
> Thanks, Jarkko.   Although it should be obvious that patch limits the
> number of times an option can be specified, you should explicitly
> mention it in the patch description.

OK, I'll update the commit message with this information before I send
the pull request. Thanks for the advice!

> Mimi

/Jarkko

> 
> > Reported-by: James Morris James Morris 
> > Signed-off-by: Jarkko Sakkinen 
> > ---
> >  security/keys/trusted.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> > index 903dace..7c183c7 100644
> > --- a/security/keys/trusted.c
> > +++ b/security/keys/trusted.c
> > @@ -736,11 +736,14 @@ static int getoptions(char *c, struct 
> > trusted_key_payload *pay,
> > int res;
> > unsigned long handle;
> > unsigned long lock;
> > +   unsigned long token_mask = 0;
> > 
> > while ((p = strsep(, " \t"))) {
> > if (*p == '\0' || *p == ' ' || *p == '\t')
> > continue;
> > token = match_token(p, key_tokens, args);
> > +   if (test_and_set_bit(token, _mask))
> > +   return -EINVAL;
> > 
> > switch (token) {
> > case Opt_pcrinfo:
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options

2015-12-14 Thread Mimi Zohar
On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote:
> The trusted keys option parsing allows specifying the same option
> multiple times. The last option value specified is used.
> 
> This can be seen as a regression because:
> 
> * No gain.
> * Could be problematic if there is be options dependent on other
>   options.

Thanks, Jarkko.   Although it should be obvious that patch limits the
number of times an option can be specified, you should explicitly
mention it in the patch description.

Mimi

> Reported-by: James Morris James Morris 
> Signed-off-by: Jarkko Sakkinen 
> ---
>  security/keys/trusted.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 903dace..7c183c7 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -736,11 +736,14 @@ static int getoptions(char *c, struct 
> trusted_key_payload *pay,
>   int res;
>   unsigned long handle;
>   unsigned long lock;
> + unsigned long token_mask = 0;
> 
>   while ((p = strsep(, " \t"))) {
>   if (*p == '\0' || *p == ' ' || *p == '\t')
>   continue;
>   token = match_token(p, key_tokens, args);
> + if (test_and_set_bit(token, _mask))
> + return -EINVAL;
> 
>   switch (token) {
>   case Opt_pcrinfo:


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html