[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <202007171502.22E12A4E9@keescook>
Date: Fri, 17 Jul 2020 15:06:12 -0700
From: Kees Cook <keescook@...omium.org>
To: Scott Branden <scott.branden@...adcom.com>
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
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.
> > 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! :)
--
Kees Cook
Powered by blists - more mailing lists