lkml.org 
[lkml]   [2006]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH] Keys: Improve usage of memory barriers and remove IRQ disablement
    Date
    The attached patch adds a missing memory barrier into the key_put() and removes
    an unnecessary barrier from install_session_keyring().

    install_session_keyring() is also rearranged a little to make it slightly more
    efficient.

    As install_*_keyring() may schedule (in synchronize_rcu() or keyring_alloc()),
    they may not be entered with interrupts disabled - and so there's no point
    saving the interrupt disablement state over the critical section.

    exec_keys() will also be invoked with interrupts enabled, and so that doesn't
    need to save the interrupt state either.
    ---

    security/keys/key.c | 1 +
    security/keys/process_keys.c | 41 ++++++++++++++++++++---------------------
    2 files changed, 21 insertions(+), 21 deletions(-)

    diff --git a/security/keys/key.c b/security/keys/key.c
    index 99781b7..d8a6e00 100644
    --- a/security/keys/key.c
    +++ b/security/keys/key.c
    @@ -619,6 +619,7 @@ void key_put(struct key *key)
    if (key) {
    key_check(key);

    + smp_mb__before_atomic_dec();
    if (atomic_dec_and_test(&key->usage))
    schedule_work(&key_cleanup_task);
    }
    diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
    index 74cb79e..ad123c3 100644
    --- a/security/keys/process_keys.c
    +++ b/security/keys/process_keys.c
    @@ -167,11 +167,12 @@ error:
    */
    int install_process_keyring(struct task_struct *tsk)
    {
    - unsigned long flags;
    struct key *keyring;
    char buf[20];
    int ret;

    + might_sleep();
    +
    if (!tsk->signal->process_keyring) {
    sprintf(buf, "_pid.%u", tsk->tgid);

    @@ -182,12 +183,12 @@ int install_process_keyring(struct task_
    }

    /* attach keyring */
    - spin_lock_irqsave(&tsk->sighand->siglock, flags);
    + spin_lock_irq(&tsk->sighand->siglock);
    if (!tsk->signal->process_keyring) {
    tsk->signal->process_keyring = keyring;
    keyring = NULL;
    }
    - spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
    + spin_unlock_irq(&tsk->sighand->siglock);

    key_put(keyring);
    }
    @@ -206,38 +207,37 @@ error:
    static int install_session_keyring(struct task_struct *tsk,
    struct key *keyring)
    {
    - unsigned long flags;
    struct key *old;
    char buf[20];
    - int ret;
    +
    + might_sleep();

    /* create an empty session keyring */
    if (!keyring) {
    sprintf(buf, "_ses.%u", tsk->tgid);

    keyring = keyring_alloc(buf, tsk->uid, tsk->gid, 1, NULL);
    - if (IS_ERR(keyring)) {
    - ret = PTR_ERR(keyring);
    - goto error;
    - }
    + if (IS_ERR(keyring))
    + return PTR_ERR(keyring);
    }
    else {
    atomic_inc(&keyring->usage);
    }

    /* install the keyring */
    - spin_lock_irqsave(&tsk->sighand->siglock, flags);
    - old = rcu_dereference(tsk->signal->session_keyring);
    + spin_lock_irq(&tsk->sighand->siglock);
    + old = tsk->signal->session_keyring;
    rcu_assign_pointer(tsk->signal->session_keyring, keyring);
    - spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
    + spin_unlock_irq(&tsk->sighand->siglock);

    - ret = 0;
    + /* we're using RCU on the pointer, but there's no point synchronising
    + * on it if it didn't previously point to anything */
    + if (old) {
    + synchronize_rcu();
    + key_put(old);
    + }

    - /* we're using RCU on the pointer */
    - synchronize_rcu();
    - key_put(old);
    -error:
    - return ret;
    + return 0;

    } /* end install_session_keyring() */

    @@ -310,7 +310,6 @@ void exit_keys(struct task_struct *tsk)
    */
    int exec_keys(struct task_struct *tsk)
    {
    - unsigned long flags;
    struct key *old;

    /* newly exec'd tasks don't get a thread keyring */
    @@ -322,10 +321,10 @@ int exec_keys(struct task_struct *tsk)
    key_put(old);

    /* discard the process keyring from a newly exec'd task */
    - spin_lock_irqsave(&tsk->sighand->siglock, flags);
    + spin_lock_irq(&tsk->sighand->siglock);
    old = tsk->signal->process_keyring;
    tsk->signal->process_keyring = NULL;
    - spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
    + spin_unlock_irq(&tsk->sighand->siglock);

    key_put(old);

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

    \
     
     \ /
      Last update: 2006-04-04 11:58    [W:0.038 / U:62.496 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site