From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B124CC433E0 for ; Fri, 17 Jul 2020 15:35:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8EB64206F4 for ; Fri, 17 Jul 2020 15:35:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="thN/THn4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726546AbgGQPfQ (ORCPT ); Fri, 17 Jul 2020 11:35:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54272 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726198AbgGQPfP (ORCPT ); Fri, 17 Jul 2020 11:35:15 -0400 Received: from mail-il1-x141.google.com (mail-il1-x141.google.com [IPv6:2607:f8b0:4864:20::141]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7CF43C0619D2; Fri, 17 Jul 2020 08:35:15 -0700 (PDT) Received: by mail-il1-x141.google.com with SMTP id e18so7691391ilr.7; Fri, 17 Jul 2020 08:35:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jWFR5o+rLcHr6gQ9i4ZRMRc2XwQllNXVeyLEXvcDyqU=; b=thN/THn4iUNzs7Zw+EhsCXiABO9WgEv1JIQa2OlXRKhXBasNjdSR+etGHN0QPWhtj3 NmNM+K5g3pqD6vzmdFUsryVGDBFX1CRqmZ27lCJLTwh+jptu5E3hYnQR7Ifh2MnHHgYe MLKlAP8gsJYvj0bhoruVQ2nM/N48lQmpaWKHYJE7QB1uB9Wp6taMWOPEGYji7zf/73+F WyWUBb3K9w9KV1S6JoUPxFcSlvIetYP8UwxXcyciNowCt9dqamlhYRP2plYJi8AjLn7m 3GCKBnA/nu3LmHozF41mEJ+laXapFqHNm8OS+2k5AxhCXW4p528BAuBps7wCs7Y1ymOG NfgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jWFR5o+rLcHr6gQ9i4ZRMRc2XwQllNXVeyLEXvcDyqU=; b=ZYk69al5zENQNe4EblfBguqH4Px7BijmFnAz5W+z3IEowINuHesDxUBOojiSSPq+Bt JYg89QEpuSNXf6pWJ8NP4t9Jhe9FOxZGD3G6U9vVmnBgU74lHItGRUPaWUPqRgWr1WSD Nm7cposUhnIYpgWM3J4nsQLGncf19cm9YMoC3/HEUpZ/v/xjuX4SDXKsPPE9JVshnvoN shdC6t4Rzd/tLLPN8GBHSRoPWx7XxvyvqayK8J6Xy+iwaMpghhSzrfi7o5VVreJM9UX7 MfULURQ5srvnDReNf4e+1gUYRAt3yxg63XP/B4+btUc8uEYZkGFMP8jR1QpW96NT89NI wWdw== X-Gm-Message-State: AOAM532QT28hSep7AxZqP23cg5lYJcaloyo44dFrHkDGjRnxn02AKrnk 9qZm/PiFRn9vI6WIPaNFPixQi9ijkhgxWXlHhFg= X-Google-Smtp-Source: ABdhPJxHE7aaEDjc30un5GGc1JQ0IJZaN/KWa3ptEgEKy/F2F3Su9D/D3HY6ZCO1lM/KMTGhQ2KEcYqpdIm1NWqjCfM= X-Received: by 2002:a92:d84d:: with SMTP id h13mr10902476ilq.102.1595000114409; Fri, 17 Jul 2020 08:35:14 -0700 (PDT) MIME-Version: 1.0 References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> <20200709061911.954326-9-tyhicks@linux.microsoft.com> In-Reply-To: <20200709061911.954326-9-tyhicks@linux.microsoft.com> From: Konsta Karsisto Date: Fri, 17 Jul 2020 18:35:03 +0300 Message-ID: Subject: Re: [PATCH v3 08/12] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements To: Tyler Hicks , linux-integrity@vger.kernel.org Cc: Mimi Zohar , Dmitry Kasatkin , James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Found one glitch with this change, see below: On Thu, Jul 9, 2020 at 9:22 AM Tyler Hicks wrote: > > The args_p member is a simple string that is allocated by > ima_rule_init(). Shallow copy it like other non-LSM references in > ima_rule_entry structs. > > There are no longer any necessary error path cleanups to do in > ima_lsm_copy_rule(). > > Signed-off-by: Tyler Hicks > --- > > * v3 > - No change > * v2 > - Adjusted context to account for ima_lsm_copy_rule() directly calling > ima_lsm_free_rule() and the lack of explicit reference ownership > transfers > - Added comment to ima_lsm_copy_rule() to document the args_p > reference ownership transfer > > security/integrity/ima/ima_policy.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 9842e2e0bc6d..b02e1ffd10c9 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -300,10 +300,13 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) > continue; > > nentry->lsm[i].type = entry->lsm[i].type; > - nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p, > - GFP_KERNEL); > - if (!nentry->lsm[i].args_p) > - goto out_err; > + nentry->lsm[i].args_p = entry->lsm[i].args_p; > + /* > + * Remove the reference from entry so that the associated > + * memory will not be freed during a later call to > + * ima_lsm_free_rule(entry). > + */ > + entry->lsm[i].args_p = NULL; This assignment necessitates a change in the code below... > security_filter_rule_init(nentry->lsm[i].type, > Audit_equal, > @@ -314,11 +317,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) > (char *)entry->lsm[i].args_p); ... you should refer to nentry->lsm[i].args_p here! Other than that, Reviewed-by: Konsta Karsisto Konsta > } > return nentry; > - > -out_err: > - ima_lsm_free_rule(nentry); > - kfree(nentry); > - return NULL; > } > > static int ima_lsm_update_rule(struct ima_rule_entry *entry) > -- > 2.25.1 >