Messages in this thread | | | Subject | Re: [PATCH 06/13] fs/kernel_read_file: Remove redundant size argument | From | Scott Branden <> | Date | Fri, 17 Jul 2020 22:44:30 -0700 |
| |
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! :) >
| |