Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <1594167871.23056.132.camel@linux.ibm.com>
Subject: Re: [PATCH v10 2/9] fs: introduce kernel_pread_file* support
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Kees Cook <keescook@chromium.org>,
        Scott Branden <scott.branden@broadcom.com>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        Wolfram Sang <wsa@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        David Brown <david.brown@linaro.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Shuah Khan <shuah@kernel.org>, bjorn.andersson@linaro.org,
        Shuah Khan <skhan@linuxfoundation.org>,
        Arnd Bergmann <arnd@arndb.de>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-fsdevel@vger.kernel.org,
        BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,
        Olof Johansson <olof@lixom.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        Colin Ian King <colin.king@canonical.com>,
        Takashi Iwai <tiwai@suse.de>, linux-kselftest@vger.kernel.org,
        Andy Gross <agross@kernel.org>,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org
Date:   Tue, 07 Jul 2020 20:24:31 -0400
In-Reply-To: <202007071642.AA705B2A@keescook>
References: <20200706232309.12010-1-scott.branden@broadcom.com>
         <20200706232309.12010-3-scott.branden@broadcom.com>
         <202007071642.AA705B2A@keescook>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 2020-07-07 at 16:56 -0700, Kees Cook wrote:
> > @@ -951,21 +955,32 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
> >               ret = -EINVAL;
> >               goto out;
> >       }
> > -     if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
> > +
> > +     /* Default read to end of file */
> > +     read_end = i_size;
> > +
> > +     /* Allow reading partial portion of file */
> > +     if ((id == READING_FIRMWARE_PARTIAL_READ) &&
> > +         (i_size > (pos + max_size)))
> > +             read_end = pos + max_size;
> 
> There's no need to involve "id" here. There are other signals about
> what's happening (i.e. pos != 0, max_size != i_size, etc).

Both the pre and post security kernel_read_file hooks are called here,
but there isn't enough information being passed to the LSM/IMA to be
able to different which hook is applicable.  One method of providing
that additional information is by enumeration.  The other option would
be to pass some additional information.

For example, on the post kernel_read_file hook, the file is read once
into memory.  IMA calculates the firmware file hash based on the
buffer contents.  On the pre kernel_read_file hook, IMA would need to
read the entire file, calculating the file hash.  Both methods of
calculating the file hash work, but the post hook is more efficient.

Mimi