Re: [RFC v2 00/10] snet: Security for NETwork syscalls

[Tetsuo Handa]

Hello.

Samir Bellabes wrote:
> Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> writes:
> 
> > Regarding [RFC v2 09/10] snet: introduce snet_ticket
> > +enum snet_verdict snet_ticket_check(struct snet_info *info)
> > +{
> > +       struct snet_ticket *st = NULL;
> > +       unsigned int h = 0, verdict = SNET_VERDICT_NONE;
> > +       struct list_head *l = NULL;
> > +       struct snet_task_security *tsec = NULL;
> > +
> > +       if (snet_ticket_mode == SNET_TICKET_OFF)
> > +               goto out;
> > +
> > +       tsec = (struct snet_task_security*) current_security();
> > +
> > +       h = jhash_2words(info->syscall, info->protocol, 0) % HSIZE;
> > +       l = &tsec->hash[h];
> > +
> > +       read_lock_bh(&tsec->lock);
> >
> > Credentials are allocated for copy-on-write basis.
> > Sharing "tsec" among multiple "struct task_struct" is what you intended?
> 
> No, there is no shared "tsec".
> snet_ticket_check() is called from the process context. So "tsec" is
> a pointer to the "void *security" pointer from its own "struct
> task_struct".
>
Until 2.6.28:

  "void *security" is directory attached to "struct task_struct".
  copy_process() calls security_task_alloc().

  Therefore, task1->security != task2->security is guaranteed as long as you do

    task->security = kmalloc();

  at security_task_alloc().

Since 2.6.29:

  "void *security" is attached to "struct cred", and "struct cred *" is
  attached to "struct task_struct". copy_process() calls copy_creds() and
  prepare_creds() calls security_prepare_creds(). But copy_creds() does not
  call prepare_creds() for clone(CLONE_THREAD) case.

  Therefore, task1->cred->security != task2->cred->security is not guaranteed
  even if you do

    cred->security = kmalloc();

  at security_prepare_creds().

> every task_struct have a "tsec" allocated to its "void *security"
> pointer. 

You meant to have assigned "void *security" dedicated to "struct task_struct".
But "void *security" is no longer directly attached to "struct task_struct".
I couldn't find code that checks whether "current->cred" is used by only
current thread or not. "current->cred" being used by only current thread is
a requirement for having a "tsec" allocated to every "struct task_struct".

Your code will share "tsec" among multiple threads if a process created
threads using clone(CLONE_THREAD | CLONE_SIGHAND | CLONE_VM). Each thread has
its own "struct task_struct" but they share "cred->security".
Sharing "tsec" among multiple threads is what you intended?



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