lkml.org 
[lkml]   [2009]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 7/7][v8] SI_USER: Masquerade si_pid when crossing pid ns boundary
    On 02/19, Eric W. Biederman wrote:
    >
    > Oleg Nesterov <oleg@redhat.com> writes:
    >
    > > On 02/19, Eric W. Biederman wrote:
    > >>
    > >> Oleg Nesterov <oleg@redhat.com> writes:
    > >> >
    > >> > SI_FROMUSER() == T, unless we have more (hopefully not) in-kernel
    > >> > users which send SI_FROMUSER() signals, .si_pid must be valid?
    > >>
    > >> So the argument is that while things such as force_sig_info(SIGSEGV)
    > >> don't have a si_pid we don't care because from_ancestor_ns == 0.
    > >>
    > >> Interesting. Then I don't know if we have any kernel senders
    > >> that cross the namespace boundaries.
    > >>
    > >> That said I still object to this code.
    > >>
    > >> sys_kill(-pgrp, SIGUSR1)
    > >> kill_something_info(SIGUSR1, &info, 0)
    > >> __kill_pgrp_info(SIGUSR1, &info task_pgrp(current))
    > >> group_send_sig_info(SIGUSR1, &info, tsk)
    > >> __group_send_sig_info(SIGUSR1, &info, tsk)
    > >> send_signal(SIGUSR1, &info, tsk, 1)
    > >> __send_signal(SIGUSR1, &info, tsk, 1)
    > >>
    > >>
    > >> Process groups and sessions can have processes in multiple pid
    > >> namespaces, which is very useful for not messing up your controlling
    > >> terminal.
    > >>
    > >> In which case sys_kill cannot possibly set the si_pid value correct
    > >> and from_ancestor_ns is not enough either.
    > >
    > > (I know, I shouldn't reply today because I am already sleeping ;)
    > >
    > > Why? send_signal() should calculate the correct value of
    > > from_parent and pass it to __send_signal(). If it is true, then
    > > we clear .si_pid in the copied siginfo (which was already queued).
    > > We don't mangle the original siginfo.
    > >
    > > This happens for each process we send the signal.
    > >
    > > Or I misunderstood you?
    >
    > Suppose I have 3 processes in a process group in three separate pid
    > namespaces.
    >
    > Looking from the init pid namespace I have:
    > pid pgrp ppid
    > 10 10 1
    > 11 10 10
    > 12 10 11
    >
    > Looking from the pid namespace of pid 11 I have:
    > pid pgrp ppid
    > 0 0 0
    > 1 0 0
    > 2 0 1
    >
    > Looking from the pid namespace of pid 12 I have:
    > pid pgrp ppid
    > 0 0 0
    > 0 0 0
    > 1 0 0
    >
    > So if the process with pid 12 in the initial pid namespace
    > sends to process group 0.

    But this is the different problem, it is not that we clear si_pid while
    we shouldn't, just the .si_pid passed from kill_something_info() is not
    right.

    Personally, I think we should not allow to send signals outside our
    namespace (except SIGCHLD on exit), this looks just wrong to me. And
    some time ago copy_process(CLONE_PID) did "setsid".

    Hmm... that was changed by your commit 5cd17569fd0eeca510735e63a6061291e3971bf6.
    And while I agree with this commit, I think that cinit should do sys_setsid()
    itself to detach itself from the parent namespace.


    Or. We can fix the case you described. We can move "si_pid = task_tgid_vnr()"
    from sys_kill/do_tkill/etc to send_signal(), it can calculate the correct
    .si_pid looking at sender/receiver namespaces.

    Oleg.



    \
     
     \ /
      Last update: 2009-02-20 01:35    [W:5.023 / U:0.680 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site