Messages in this thread | | | Date | Mon, 09 Jun 2014 11:17:08 +0300 | From | Dmitry Kasatkin <> | Subject | Re: [RFC PATCH v5 1/4] KEYS: special dot prefixed keyring name bug fix |
| |
On 09/06/14 10:56, Dmitry Kasatkin wrote: > On 07/06/14 01:00, Mimi Zohar wrote: >> On Sat, 2014-06-07 at 00:48 +0300, Dmitry Kasatkin wrote: >>> On 3 June 2014 20:58, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >>>> Dot prefixed keyring names are supposed to be reserved for the >>>> kernel, but add_key() calls key_get_type_from_user(), which >>>> incorrectly verifies the 'type' field, not the 'description' field. >>>> This patch verifies the 'description' field isn't dot prefixed, >>>> when creating a new keyring, and removes the dot prefix test in >>>> key_get_type_from_user(). >>>> >>>> Changelog v5: >>>> - Only prevent userspace from creating a dot prefixed keyring, not >>>> regular keys - Dmitry >>>> >>>> Reported-by: Dmitry Kasatkin <d.kasatkin@samsung.com> >>>> Cc: David Howells <dhowells@redhat.com> >>>> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> >>>> --- >>>> security/keys/keyctl.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c >>>> index cd5bd0c..62a9952 100644 >>>> --- a/security/keys/keyctl.c >>>> +++ b/security/keys/keyctl.c >>>> @@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type, >>>> return ret; >>>> if (ret == 0 || ret >= len) >>>> return -EINVAL; >>>> - if (type[0] == '.') >>>> - return -EPERM; >>>> type[len - 1] = '\0'; >>>> return 0; >>>> } >>>> @@ -86,6 +84,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type, >>>> if (!*description) { >>>> kfree(description); >>>> description = NULL; >>>> + } else if ((description[0] == '.') && >>>> + (strncmp(type, "keyring", 7) == 0)) {
BTW. using strcmp is good enough here. string constants are always NULL terminated. Comparison will terminate after no more than 7 bytes.
>>>> + ret = -EPERM; >>>> + goto error2; >>>> } >>>> } >>> I think it does not another problem... >>> It is not only prevent creating new keyring with ".abc" name but also >>> prevent adding new key... >>> >>> this is wrong... >> Seems to prevent creating a dot prefixed keyring, but permits creating a >> dot prefixed key. Do you have an example? > I think by mistake I was checking old patch in the thread because I have > not noticed strncmp(type, "keyring", 7). > This patch definitely should do the job. > > - Dmitry > > >> thanks, >> >> Mimi >> >> Mimi >> >> >>
| |