linux-kernel - Re: [PATCH 06/13] fs/kernel_read_file: Remove redundant size argument

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <99c58d32-da28-79ad-2fdb-83e0ca29660a@broadcom.com>
Date:   Fri, 17 Jul 2020 22:44:30 -0700
From:   Scott Branden <scott.branden@...adcom.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Mimi Zohar <zohar@...ux.ibm.com>,
        Matthew Wilcox <willy@...radead.org>,
        James Morris <jmorris@...ei.org>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Jessica Yu <jeyu@...nel.org>,
        Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Matthew Garrett <matthewgarrett@...gle.com>,
        David Howells <dhowells@...hat.com>,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        "Joel Fernandes (Google)" <joel@...lfernandes.org>,
        KP Singh <kpsingh@...gle.com>, Dave Olsthoorn <dave@...aar.me>,
        Hans de Goede <hdegoede@...hat.com>,
        Peter Jones <pjones@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Stephen Boyd <stephen.boyd@...aro.org>,
        Paul Moore <paul@...l-moore.com>,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        linux-security-module@...r.kernel.org,
        linux-integrity@...r.kernel.org, selinux@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, kexec@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/13] fs/kernel_read_file: Remove redundant size argument

Hi Kees,

On 2020-07-17 3:06 p.m., Kees Cook wrote:
> On Fri, Jul 17, 2020 at 12:04:18PM -0700, Scott Branden wrote:
>> On 2020-07-17 10:43 a.m., Kees Cook wrote:
>>> In preparation for refactoring kernel_read_file*(), remove the redundant
>>> "size" argument which is not needed: it can be included in the return
>> I don't think the size argument is redundant though.
>> The existing kernel_read_file functions always read the whole file.
>> Now, what happens if the file is bigger than the buffer.
>> How does kernel_read_file know it read the whole file by looking at the
>> return value?
> Yes; an entirely reasonable concern. This is why I add the file_size
> output argument later in the series.
There is something wrong with this patch.  I apply patches 1-5 and these 
pass the kernel self test.
Patch 6 does not pass the kernel-selftest/firmware/fw_run_tests.sh

>>> code, with callers adjusted. (VFS reads already cannot be larger than
>>> INT_MAX.)
>>> [...]
>>> -	if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
>>> +	if (i_size > INT_MAX || (max_size > 0 && i_size > max_size)) {
>> Should this be SSIZE_MAX?
> No, for two reasons: then we need to change the return value and likely
> the callers need more careful checks, and more importantly, because the
> VFS already limits single read actions to INT_MAX, so limits above this
> make no sense. Win win! :)
>