Skip to content

Commit e7080a4

Browse files
nicstangetorvalds
authored andcommitted
mm/filemap: generic_file_read_iter(): check for zero reads unconditionally
If - generic_file_read_iter() gets called with a zero read length, - the read offset is at a page boundary, - IOCB_DIRECT is not set - and the page in question hasn't made it into the page cache yet, then do_generic_file_read() will trigger a readahead with a req_size hint of zero. Since roundup_pow_of_two(0) is undefined, UBSAN reports UBSAN: Undefined behaviour in include/linux/log2.h:63:13 shift exponent 64 is too large for 64-bit type 'long unsigned int' CPU: 3 PID: 1017 Comm: sa1 Tainted: G L 4.5.0-next-20160318+ #14 [...] Call Trace: [...] [<ffffffff813ef61a>] ondemand_readahead+0x3aa/0x3d0 [<ffffffff813ef61a>] ? ondemand_readahead+0x3aa/0x3d0 [<ffffffff813c73bd>] ? find_get_entry+0x2d/0x210 [<ffffffff813ef9c3>] page_cache_sync_readahead+0x63/0xa0 [<ffffffff813cc04d>] do_generic_file_read+0x80d/0xf90 [<ffffffff813cc955>] generic_file_read_iter+0x185/0x420 [...] [<ffffffff81510b06>] __vfs_read+0x256/0x3d0 [...] when get_init_ra_size() gets called from ondemand_readahead(). The net effect is that the initial readahead size is arch dependent for requested read lengths of zero: for example, since 1UL << (sizeof(unsigned long) * 8) evaluates to 1 on x86 while its result is 0 on ARMv7, the initial readahead size becomes 4 on the former and 0 on the latter. What's more, whether or not the file access timestamp is updated for zero length reads is decided differently for the two cases of IOCB_DIRECT being set or cleared: in the first case, generic_file_read_iter() explicitly skips updating that timestamp while in the latter case, it is always updated through the call to do_generic_file_read(). According to POSIX, zero length reads "do not modify the last data access timestamp" and thus, the IOCB_DIRECT behaviour is POSIXly correct. Let generic_file_read_iter() unconditionally check the requested read length at its entry and return immediately with success if it is zero. Signed-off-by: Nicolai Stange <[email protected]> Cc: Al Viro <[email protected]> Reviewed-by: Jan Kara <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 9dcadd3 commit e7080a4

File tree

1 file changed

+4
-3
lines changed

1 file changed

+4
-3
lines changed

mm/filemap.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1840,15 +1840,16 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
18401840
ssize_t retval = 0;
18411841
loff_t *ppos = &iocb->ki_pos;
18421842
loff_t pos = *ppos;
1843+
size_t count = iov_iter_count(iter);
1844+
1845+
if (!count)
1846+
goto out; /* skip atime */
18431847

18441848
if (iocb->ki_flags & IOCB_DIRECT) {
18451849
struct address_space *mapping = file->f_mapping;
18461850
struct inode *inode = mapping->host;
1847-
size_t count = iov_iter_count(iter);
18481851
loff_t size;
18491852

1850-
if (!count)
1851-
goto out; /* skip atime */
18521853
size = i_size_read(inode);
18531854
retval = filemap_write_and_wait_range(mapping, pos,
18541855
pos + count - 1);

0 commit comments

Comments
 (0)