Oops in call_sbin_request_key in 2.6.26-rc2

[Steve French-2]

Could this changeset be the culprit?

commit 69664cf16af4f31cd54d77948a4baf9c7e0ca7b9
Author: David Howells <dhowells@...>
Date:   Tue Apr 29 01:01:31 2008 -0700

    keys: don't generate user and user session keyrings unless they're accessed


since it seems to change the way sesion_keyrings are allocated?

On Mon, May 19, 2008 at 12:34 PM, Steve French <smfrench@...> wrote:

> Oops is caused at the line in security/keys/request_key.c in
> call_sbin_request_key:
>      sskey = tsk->user->session_keyring->serial;
> because session_keyring is null.
>
> On Sun, May 18, 2008 at 8:21 PM, Steve French <smfrench@...> wrote:
>> With the attached path (which only fixes the UnixQueryPathInfo case),
>> am oopsing in the upcall on samba localhost
>
>> BUG: unable to handle kernel NULL pointer dereference at 00000004
>> IP: [<c02b3c8c>] call_sbin_request_key+0x148/0x220
>> *pde = 00000000
>> Oops: 0000 [#1] SMP
>> Modules linked in: cifs
>>
>> Pid: 6236, comm: bash Not tainted (2.6.26-rc2-00052-gd0a9c07-dirty #4)
>> EIP: 0060:[<c02b3c8c>] EFLAGS: 00010246 CPU: 0
>> EIP is at call_sbin_request_key+0x148/0x220
>> EAX: 00000000 EBX: 00000000 ECX: fffffffb EDX: deb99bc6
>> ESI: f675a5d0 EDI: deb99ca0 EBP: deb99d2c ESP: deb99c84
>>  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
>> Process bash (pid: 6236, ti=deb98000 task=f675a5d0 task.ti=deb98000)
>> Stack: c0635239 f7befd08 deb87580 deb87380 deb80030 deb99ca8 deb87588 deb90030
>>       c04e7427 deb99cd8 00000000 deb99ccc c02b0c36 f6c3a800 deb87ac4 00000000
>>       00000000 f6c3a800 deb99cec c02b0c7f 00000000 00000000 7165725f 3730372e
>> Call Trace:
>>  [<c04e7427>] ? mutex_lock+0xe/0x1e
>>  [<c02b0c36>] ? __key_instantiate_and_link+0x8b/0xa5
>>  [<c02b0c7f>] ? key_instantiate_and_link+0x2f/0x49
>>  [<c02b0030>] ? mqueue_poll_file+0x1d/0x56
>>  [<c02b3b44>] ? call_sbin_request_key+0x0/0x220
>>  [<c02b3a27>] ? request_key_and_link+0x1e3/0x231
>>  [<c02b3d8f>] ? request_key+0x2b/0x58
>>  [<f8d38bc7>] ? dns_resolve_server_name_to_ip+0x1f6/0x217 [cifs]
>>  [<f8d38eff>] ? cifs_dfs_follow_mountpoint+0x2bc/0x631 [cifs]
>>  [<c01673ac>] ? do_lookup+0x4f/0x140
>>  [<c0173bea>] ? mnt_drop_write+0x1d/0xb7
>>  [<c0168b02>] ? __link_path_walk+0x81c/0xb0b
>>  [<c011f9a2>] ? printk+0x15/0x17
>>  [<c0168e3d>] ? path_walk+0x4c/0x9b
>>  [<c01690dc>] ? do_path_lookup+0x11f/0x13a
>>  [<c01698fa>] ? __user_walk_fd+0x2f/0x48
>>  [<c0163e3d>] ? vfs_stat_fd+0x19/0x40
>>  [<c0163f13>] ? vfs_stat+0x11/0x13
>>  [<c0163f29>] ? sys_stat64+0x14/0x28
>>  [<c01036c9>] ? sysenter_past_esp+0x6a/0x91
>>  =======================
>> Code: ff ff 57 e8 6a 6c 01 00 8b 86 ac 02 00 00 83 c4 0c 83 b8 a0 01
>> 00 00 00 74 08 8b 80 a0 01 00 00 eb 09 8b 86 e4 01 00 00 8b 40 24 <8b>
>> 40 04 8d 5d 80 50 68 96 5b 64 c0 53 e8 35 6c 01 00 8b 85 58
>> EIP: [<c02b3c8c>] call_sbin_request_key+0x148/0x220 SS:ESP 0068:deb99c84
>> ---[ end trace 2cb388b698ff26eb ]---
>
>
>
> --
> Thanks,
>
> Steve
>

--
Thanks,

Steve
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Dave Kleikamp-3]

Adding dhowells.  Was that an oversight?

On Mon, 2008-05-19 at 12:45 -0500, Steve French wrote:
> Could this changeset be the culprit?

It sure looks like it.

> commit 69664cf16af4f31cd54d77948a4baf9c7e0ca7b9
> Author: David Howells <dhowells@...>
> Date:   Tue Apr 29 01:01:31 2008 -0700
>
>     keys: don't generate user and user session keyrings unless they're accessed
>
>
> since it seems to change the way sesion_keyrings are allocated?

David,
It looks like call_sbin_request_key() needs to handle
tsk->user->session_keyring being null.

Shaggy

> On Mon, May 19, 2008 at 12:34 PM, Steve French <smfrench@...> wrote:
> > Oops is caused at the line in security/keys/request_key.c in
> > call_sbin_request_key:
> >      sskey = tsk->user->session_keyring->serial;
> > because session_keyring is null.
> >
> > On Sun, May 18, 2008 at 8:21 PM, Steve French <smfrench@...> wrote:
> >> With the attached path (which only fixes the UnixQueryPathInfo case),
> >> am oopsing in the upcall on samba localhost
> >
> >> BUG: unable to handle kernel NULL pointer dereference at 00000004
> >> IP: [<c02b3c8c>] call_sbin_request_key+0x148/0x220
> >> *pde = 00000000
> >> Oops: 0000 [#1] SMP
> >> Modules linked in: cifs
> >>
> >> Pid: 6236, comm: bash Not tainted (2.6.26-rc2-00052-gd0a9c07-dirty #4)
> >> EIP: 0060:[<c02b3c8c>] EFLAGS: 00010246 CPU: 0
> >> EIP is at call_sbin_request_key+0x148/0x220
> >> EAX: 00000000 EBX: 00000000 ECX: fffffffb EDX: deb99bc6
> >> ESI: f675a5d0 EDI: deb99ca0 EBP: deb99d2c ESP: deb99c84
> >>  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> >> Process bash (pid: 6236, ti=deb98000 task=f675a5d0 task.ti=deb98000)
> >> Stack: c0635239 f7befd08 deb87580 deb87380 deb80030 deb99ca8 deb87588 deb90030
> >>       c04e7427 deb99cd8 00000000 deb99ccc c02b0c36 f6c3a800 deb87ac4 00000000
> >>       00000000 f6c3a800 deb99cec c02b0c7f 00000000 00000000 7165725f 3730372e
> >> Call Trace:
> >>  [<c04e7427>] ? mutex_lock+0xe/0x1e
> >>  [<c02b0c36>] ? __key_instantiate_and_link+0x8b/0xa5
> >>  [<c02b0c7f>] ? key_instantiate_and_link+0x2f/0x49
> >>  [<c02b0030>] ? mqueue_poll_file+0x1d/0x56
> >>  [<c02b3b44>] ? call_sbin_request_key+0x0/0x220
> >>  [<c02b3a27>] ? request_key_and_link+0x1e3/0x231
> >>  [<c02b3d8f>] ? request_key+0x2b/0x58
> >>  [<f8d38bc7>] ? dns_resolve_server_name_to_ip+0x1f6/0x217 [cifs]
> >>  [<f8d38eff>] ? cifs_dfs_follow_mountpoint+0x2bc/0x631 [cifs]
> >>  [<c01673ac>] ? do_lookup+0x4f/0x140
> >>  [<c0173bea>] ? mnt_drop_write+0x1d/0xb7
> >>  [<c0168b02>] ? __link_path_walk+0x81c/0xb0b
> >>  [<c011f9a2>] ? printk+0x15/0x17
> >>  [<c0168e3d>] ? path_walk+0x4c/0x9b
> >>  [<c01690dc>] ? do_path_lookup+0x11f/0x13a
> >>  [<c01698fa>] ? __user_walk_fd+0x2f/0x48
> >>  [<c0163e3d>] ? vfs_stat_fd+0x19/0x40
> >>  [<c0163f13>] ? vfs_stat+0x11/0x13
> >>  [<c0163f29>] ? sys_stat64+0x14/0x28
> >>  [<c01036c9>] ? sysenter_past_esp+0x6a/0x91
> >>  =======================
> >> Code: ff ff 57 e8 6a 6c 01 00 8b 86 ac 02 00 00 83 c4 0c 83 b8 a0 01
> >> 00 00 00 74 08 8b 80 a0 01 00 00 eb 09 8b 86 e4 01 00 00 8b 40 24 <8b>
> >> 40 04 8d 5d 80 50 68 96 5b 64 c0 53 e8 35 6c 01 00 8b 85 58
> >> EIP: [<c02b3c8c>] call_sbin_request_key+0x148/0x220 SS:ESP 0068:deb99c84
> >> ---[ end trace 2cb388b698ff26eb ]---
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve

--
David Kleikamp
IBM Linux Technology Center

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Steve French-2]

Yes - that makes sense that it should check - search_process_keyrings
already checks for this (null user keyring):
ie
        /* or search the user-session keyring */
        else if (context->user->session_keyring) {

but the callout via request_key does not

On Mon, May 19, 2008 at 1:02 PM, Dave Kleikamp
<shaggy@...> wrote:

> Adding dhowells.  Was that an oversight?
>
> On Mon, 2008-05-19 at 12:45 -0500, Steve French wrote:
>> Could this changeset be the culprit?
>
> It sure looks like it.
>
>> commit 69664cf16af4f31cd54d77948a4baf9c7e0ca7b9
>> Author: David Howells <dhowells@...>
>> Date:   Tue Apr 29 01:01:31 2008 -0700
>>
>>     keys: don't generate user and user session keyrings unless they're accessed
>>
>>
>> since it seems to change the way sesion_keyrings are allocated?
>
> David,
> It looks like call_sbin_request_key() needs to handle
> tsk->user->session_keyring being null.
>
> Shaggy
>
>> On Mon, May 19, 2008 at 12:34 PM, Steve French <smfrench@...> wrote:
>> > Oops is caused at the line in security/keys/request_key.c in
>> > call_sbin_request_key:
>> >      sskey = tsk->user->session_keyring->serial;
>> > because session_keyring is null.
>> >
>> > On Sun, May 18, 2008 at 8:21 PM, Steve French <smfrench@...> wrote:
>> >> With the attached path (which only fixes the UnixQueryPathInfo case),
>> >> am oopsing in the upcall on samba localhost
>> >
>> >> BUG: unable to handle kernel NULL pointer dereference at 00000004
>> >> IP: [<c02b3c8c>] call_sbin_request_key+0x148/0x220
>> >> *pde = 00000000
>> >> Oops: 0000 [#1] SMP
>> >> Modules linked in: cifs
>> >>
>> >> Pid: 6236, comm: bash Not tainted (2.6.26-rc2-00052-gd0a9c07-dirty #4)
>> >> EIP: 0060:[<c02b3c8c>] EFLAGS: 00010246 CPU: 0
>> >> EIP is at call_sbin_request_key+0x148/0x220
>> >> EAX: 00000000 EBX: 00000000 ECX: fffffffb EDX: deb99bc6
>> >> ESI: f675a5d0 EDI: deb99ca0 EBP: deb99d2c ESP: deb99c84
>> >>  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
>> >> Process bash (pid: 6236, ti=deb98000 task=f675a5d0 task.ti=deb98000)
>> >> Stack: c0635239 f7befd08 deb87580 deb87380 deb80030 deb99ca8 deb87588 deb90030
>> >>       c04e7427 deb99cd8 00000000 deb99ccc c02b0c36 f6c3a800 deb87ac4 00000000
>> >>       00000000 f6c3a800 deb99cec c02b0c7f 00000000 00000000 7165725f 3730372e
>> >> Call Trace:
>> >>  [<c04e7427>] ? mutex_lock+0xe/0x1e
>> >>  [<c02b0c36>] ? __key_instantiate_and_link+0x8b/0xa5
>> >>  [<c02b0c7f>] ? key_instantiate_and_link+0x2f/0x49
>> >>  [<c02b0030>] ? mqueue_poll_file+0x1d/0x56
>> >>  [<c02b3b44>] ? call_sbin_request_key+0x0/0x220
>> >>  [<c02b3a27>] ? request_key_and_link+0x1e3/0x231
>> >>  [<c02b3d8f>] ? request_key+0x2b/0x58
>> >>  [<f8d38bc7>] ? dns_resolve_server_name_to_ip+0x1f6/0x217 [cifs]
>> >>  [<f8d38eff>] ? cifs_dfs_follow_mountpoint+0x2bc/0x631 [cifs]
>> >>  [<c01673ac>] ? do_lookup+0x4f/0x140
>> >>  [<c0173bea>] ? mnt_drop_write+0x1d/0xb7
>> >>  [<c0168b02>] ? __link_path_walk+0x81c/0xb0b
>> >>  [<c011f9a2>] ? printk+0x15/0x17
>> >>  [<c0168e3d>] ? path_walk+0x4c/0x9b
>> >>  [<c01690dc>] ? do_path_lookup+0x11f/0x13a
>> >>  [<c01698fa>] ? __user_walk_fd+0x2f/0x48
>> >>  [<c0163e3d>] ? vfs_stat_fd+0x19/0x40
>> >>  [<c0163f13>] ? vfs_stat+0x11/0x13
>> >>  [<c0163f29>] ? sys_stat64+0x14/0x28
>> >>  [<c01036c9>] ? sysenter_past_esp+0x6a/0x91
>> >>  =======================
>> >> Code: ff ff 57 e8 6a 6c 01 00 8b 86 ac 02 00 00 83 c4 0c 83 b8 a0 01
>> >> 00 00 00 74 08 8b 80 a0 01 00 00 eb 09 8b 86 e4 01 00 00 8b 40 24 <8b>
>> >> 40 04 8d 5d 80 50 68 96 5b 64 c0 53 e8 35 6c 01 00 8b 85 58
>> >> EIP: [<c02b3c8c>] call_sbin_request_key+0x148/0x220 SS:ESP 0068:deb99c84
>> >> ---[ end trace 2cb388b698ff26eb ]---
>> >
>> >
>> >
>> > --
>> > Thanks,
>> >
>> > Steve
> --
> David Kleikamp
> IBM Linux Technology Center
>
>

--
Thanks,

Steve
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[David Howells]

Dave Kleikamp <shaggy@...> wrote:

> It looks like call_sbin_request_key() needs to handle
> tsk->user->session_keyring being null.

Oops.  Yes, that looks like a good candidate for the problem.

David
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[David Howells]

Steve French <smfrench@...> wrote:

> Yes - that makes sense that it should check - search_process_keyrings
> already checks for this (null user keyring):
> ie
>         /* or search the user-session keyring */
>         else if (context->user->session_keyring) {
>
> but the callout via request_key does not

Can you try the attached patch please?  I don't have access to my test machine
at the moment, so I haven't tested it.

David
---
KEYS: Make request key instantiate the per-user keyrings

Make request_key() instantiate the per-user keyrings so that it doesn't oops
if it needs to get hold of the user session keyring because there isn't a
session keyring in place.

Signed-off-by: David Howells <dhowells@...>
---

 security/keys/internal.h     |    1 +
 security/keys/process_keys.c |    2 +-
 security/keys/request_key.c  |    4 ++++
 3 files changed, 6 insertions(+), 1 deletions(-)


diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8c05587..2bdfacc 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -108,6 +108,7 @@ extern key_ref_t search_process_keyrings(struct key_type *type,
 
 extern struct key *find_keyring_by_name(const char *name, bool skip_perm_check);
 
+extern int install_user_keyrings(struct task_struct *tsk);
 extern int install_thread_keyring(struct task_struct *tsk);
 extern int install_process_keyring(struct task_struct *tsk);
 
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 5be6d01..45b240a 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -40,7 +40,7 @@ struct key_user root_key_user = {
 /*
  * install user and user session keyrings for a particular UID
  */
-static int install_user_keyrings(struct task_struct *tsk)
+int install_user_keyrings(struct task_struct *tsk)
 {
  struct user_struct *user = tsk->user;
  struct key *uid_keyring, *session_keyring;
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index ba32ca6..abea08f 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -74,6 +74,10 @@ static int call_sbin_request_key(struct key_construction *cons,
 
  kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
 
+ ret = install_user_keyrings(tsk);
+ if (ret < 0)
+ goto error_alloc;
+
  /* allocate a new session keyring */
  sprintf(desc, "_req.%u", key->serial);
 
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Steve French-2]

With the patch - no more oops in the key API

On Tue, May 20, 2008 at 10:50 AM, David Howells <dhowells@...> wrote:

> Steve French <smfrench@...> wrote:
>
>> Yes - that makes sense that it should check - search_process_keyrings
>> already checks for this (null user keyring):
>> ie
>>         /* or search the user-session keyring */
>>         else if (context->user->session_keyring) {
>>
>> but the callout via request_key does not
>
> Can you try the attached patch please?  I don't have access to my test machine
> at the moment, so I haven't tested it.
>
> David
> ---
> KEYS: Make request key instantiate the per-user keyrings
>
> Make request_key() instantiate the per-user keyrings so that it doesn't oops
> if it needs to get hold of the user session keyring because there isn't a
> session keyring in place.
>
> Signed-off-by: David Howells <dhowells@...>
> ---
>
>  security/keys/internal.h     |    1 +
>  security/keys/process_keys.c |    2 +-
>  security/keys/request_key.c  |    4 ++++
>  3 files changed, 6 insertions(+), 1 deletions(-)
>
>
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 8c05587..2bdfacc 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -108,6 +108,7 @@ extern key_ref_t search_process_keyrings(struct key_type *type,
>
>  extern struct key *find_keyring_by_name(const char *name, bool skip_perm_check);
>
> +extern int install_user_keyrings(struct task_struct *tsk);
>  extern int install_thread_keyring(struct task_struct *tsk);
>  extern int install_process_keyring(struct task_struct *tsk);
>
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index 5be6d01..45b240a 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -40,7 +40,7 @@ struct key_user root_key_user = {
>  /*
>  * install user and user session keyrings for a particular UID
>  */
> -static int install_user_keyrings(struct task_struct *tsk)
> +int install_user_keyrings(struct task_struct *tsk)
>  {
>        struct user_struct *user = tsk->user;
>        struct key *uid_keyring, *session_keyring;
> diff --git a/security/keys/request_key.c b/security/keys/request_key.c
> index ba32ca6..abea08f 100644
> --- a/security/keys/request_key.c
> +++ b/security/keys/request_key.c
> @@ -74,6 +74,10 @@ static int call_sbin_request_key(struct key_construction *cons,
>
>        kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
>
> +       ret = install_user_keyrings(tsk);
> +       if (ret < 0)
> +               goto error_alloc;
> +
>        /* allocate a new session keyring */
>        sprintf(desc, "_req.%u", key->serial);
>
>

--
Thanks,

Steve
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client