lkml.org 
[lkml]   [2021]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH] ima: fix deadlock when traversing "ima_default_rules".
    Date
    The current IMA ruleset is identified by the variable "ima_rules"
    that default to "&ima_default_rules". When loading a custom policy
    for the first time, the variable is updated to "&ima_policy_rules"
    instead. That update isn't RCU-safe, and deadlocks are possible.
    Indeed, some functions like ima_match_policy() may loop indefinitely
    when traversing "ima_default_rules" with list_for_each_entry_rcu().

    When iterating over the default ruleset back to head, if the list
    head is "ima_default_rules", and "ima_rules" have been updated to
    "&ima_policy_rules", the loop condition (&entry->list != ima_rules)
    stays always true, traversing won't terminate, causing a soft lockup
    and RCU stalls.

    Introduce a temporary value for "ima_rules" when iterating over
    the ruleset to avoid the deadlocks.

    Signed-off-by: liqiong <liqiong@nfschina.com>
    Reviewed-by: THOBY Simon <Simon.THOBY@viveris.fr>
    ---
    security/integrity/ima/ima_policy.c | 17 ++++++++++++-----
    1 file changed, 12 insertions(+), 5 deletions(-)

    diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
    index fd5d46e511f1..e92b197bfd3c 100644
    --- a/security/integrity/ima/ima_policy.c
    +++ b/security/integrity/ima/ima_policy.c
    @@ -662,12 +662,14 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
    {
    struct ima_rule_entry *entry;
    int action = 0, actmask = flags | (flags << 1);
    + struct list_head *ima_rules_tmp;

    if (template_desc && !*template_desc)
    *template_desc = ima_template_desc_current();

    rcu_read_lock();
    - list_for_each_entry_rcu(entry, ima_rules, list) {
    + ima_rules_tmp = rcu_dereference(ima_rules);
    + list_for_each_entry_rcu(entry, ima_rules_tmp, list) {

    if (!(entry->action & actmask))
    continue;
    @@ -919,8 +921,8 @@ void ima_update_policy(void)

    if (ima_rules != policy) {
    ima_policy_flag = 0;
    - ima_rules = policy;

    + rcu_assign_pointer(ima_rules, policy);
    /*
    * IMA architecture specific policy rules are specified
    * as strings and converted to an array of ima_entry_rules
    @@ -1649,9 +1651,11 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos)
    {
    loff_t l = *pos;
    struct ima_rule_entry *entry;
    + struct list_head *ima_rules_tmp;

    rcu_read_lock();
    - list_for_each_entry_rcu(entry, ima_rules, list) {
    + ima_rules_tmp = rcu_dereference(ima_rules);
    + list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
    if (!l--) {
    rcu_read_unlock();
    return entry;
    @@ -1670,7 +1674,8 @@ void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos)
    rcu_read_unlock();
    (*pos)++;

    - return (&entry->list == ima_rules) ? NULL : entry;
    + return (&entry->list == &ima_default_rules ||
    + &entry->list == &ima_policy_rules) ? NULL : entry;
    }

    void ima_policy_stop(struct seq_file *m, void *v)
    @@ -1872,6 +1877,7 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
    struct ima_rule_entry *entry;
    bool found = false;
    enum ima_hooks func;
    + struct list_head *ima_rules_tmp;

    if (id >= READING_MAX_ID)
    return false;
    @@ -1879,7 +1885,8 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
    func = read_idmap[id] ?: FILE_CHECK;

    rcu_read_lock();
    - list_for_each_entry_rcu(entry, ima_rules, list) {
    + ima_rules_tmp = rcu_dereference(ima_rules);
    + list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
    if (entry->action != APPRAISE)
    continue;

    --
    2.11.0
    \
     
     \ /
      Last update: 2021-08-27 12:38    [W:4.275 / U:0.216 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site