在 2018/1/2 22:31, Stephen Smalley 写道:


On Jan 2, 2018 1:37 AM, "Li Kun" <[email protected] <mailto:[email protected]>> wrote:



    On 2018/1/2 12:16, Stephen Smalley wrote:
    On Jan 1, 2018 8:40 PM, "Li Kun" <[email protected]
    <mailto:[email protected]>> wrote:



        在 2017/12/30 1:25, Stephen Smalley 写道:
        On Dec 28, 2017 10:14 PM, "Li Kun" <[email protected]
        <mailto:[email protected]>> wrote:



            在 2017/12/28 22:57, Stephen Smalley 写道:
            On Sat, Dec 23, 2017 at 3:03 AM, Li Kun
            <[email protected] <mailto:[email protected]>> wrote:

                Hi all,
                When i start a docker container, the runc will call
                selinux_setprocattr to set the exec_sid before
                start the container.
                Meanwhile if i use "semodule -i" to load a policy
                pp, the old sidtab will be shutdown before switch
                to the new sidtab, and cause
                sidtab_context_to_sid  failed with the errno -NOMEM.

                Error message:
                Dec 14 09:18:18 wuwenming_vudfua_0 docker:
                time="2017-12-14T09:18:18.549862564+08:00"
                level=error msg="Handler for GET
                
/v1.23/containers/192.168.0.2:9082/dfs_d:V100R013C11SPC000BCD2/json
                <http://192.168.0.2:9082/dfs_d:V100R013C11SPC000BCD2/json>
                returned error: No such container:
                192.168.0.2:9082/dfs_d:V100R013C11SPC000BCD2
                <http://192.168.0.2:9082/dfs_d:V100R013C11SPC000BCD2>"
                Dec 14 09:18:18 wuwenming_vudfua_0 kernel: [
                48.463179] SELinux:
                
security_context_to_sid(system_u:object_r:svirt_sandbox_file_t:s0:c290,c371)
                failed for (dev overlay, type overlay) errno=-12
                Dec 14 09:18:18 wuwenming_vudfua_0 dbus-daemon:
                dbus[720]: avc:  received policyload notice (seqno=2)
Dec 14 09:18:18 wuwenming_vudfua_0 dbus[720]: avc: received policyload notice (seqno=2)


                runc semodule
                ->selinux_setprocattr ->security_load_policy
                ->security_context_to_sid ->sidtab_shutdown(oldsidtab)
                ->read_lock(&policy_rwlock);
                ->sidtab_map(&oldsidtab, clone_sid, &newsidtab);

                ->sidtab_context_to_sid
                            {
                                        ....
                                        if (s->next_sid == UINT_MAX
                || s->shutdown) {
                                        ret = -ENOMEM;
                                        ....
                            }
                ->read_unlock(&policy_rwlock);
                ->write_lock_irq(&policy_rwlock);
                ->"switch to new policydb"
                ->write_unlock_irq(&policy_rwlock);

                I wonder that if this is the intention or a bug?
                If it is the intention, what should the application
                do when it get -ENOMEM error,  to try again?
                If it is a bug, may the two options below suitable
                to solve the issue?
                Option1:
                    use policy_rwlock to protect the
                "sidtab_shutdown" & "sidtab_map" , load_policy is
                rarely to be called after system bringing up,
                so i think it will not impact much on the performance.
                Option2:
                    Use a temp list to store the sid in oldsidtab
                after it is shutdown, and deal with it
                after cloning the major sids from oldsidtab to
                newsidtab and getting the policy_rwlock.


            The current logic is intentional. One might argue that
            a different error code might be more suitable in this
            situation (e.g. EAGAIN or something), but ENOMEM is
            already a possible error code when loading policy upon
            transient OOM conditions, so the caller might have to
            retry regardless. The policy write lock must only be
            held for the minimal critical section as otherwise all
            system activity could be blocked; it is only held when
            making the new policydb and sidtab active not during
            their allocation and setup - ideally it would be
            further reduced to a couple of pointer updates. Is this
            scenario something you are encountering in actual
            practice or just a theoretically possible one?
            Thank you very much for your reply.
            Yes, we encounter this issue in our implementation.
            We have an intertion in our design that we can divide
            the policy into several parts to load separately.
            So after the red-hat linux bringing up, which has loaded
            the system policy db released by redhat, we begin to
            load the policy released by our product ,
            and meanwhile someone want to start a docker container,
            and the runc get an ENOMEM error like i mentioned above.
            Should we merge all the policy into the system policy db
            and just load once?


-- Best Regards
            Li Kun

        Typically you would install your policy module when your
        package is installed and not need to do it at runtime.
        Would you think it's suibable to change the error code to
        EAGAIN?


    You likely want it to return - EINTR or -ERESTARTSYS if you want
    existing callers of setprocattr to automatically restart the call
    instead of failing.
    AFAIU, the EINTR & ERESTARTSYS are releated to sigpending under
    most circumstances, is it proper to use them here?
    And as i mentioned above, most apps(like runc, bind)do not retry
    when they meet ENOMEM, so i think maybe EAGAIN is more suitable
    for the transient race conditions


EINTR is already handled by the libselinux procattr code, so returning that will cause existing users of the libselinux interfaces to retry. EAGAIN is not handled since the descriptor is not marked nonblocking.
Ok, i understand, thank you.
Maybe i can send a patch to change the errno to EINTR if you think it make sense.

--
Best Regards
Li Kun

Reply via email to