Re: [PATCH v5 33/40] netfs, cachefiles: Pass upper bound length to allow expansion
From: Gao Xiang <hidden>
Date: 2024-01-02 20:37:34
Also in:
ceph-devel, linux-cifs, linux-fsdevel, linux-mm, linux-nfs, lkml, v9fs
On 2024/1/3 01:11, David Howells wrote:
Gao Xiang [off-list ref] wrote:quoted
quoted
down = start - round_down(start, PAGE_SIZE); *_start = start - down; *_len = round_up(down + len, PAGE_SIZE); + if (down < start || *_len > upper_len) + return -ENOBUFS;Sorry for bothering. We just found some strange when testing today-next EROFS over fscache. I'm not sure the meaning of if (down < start For example, if start is page-aligned, down == 0. so as long as start > 0 and page-aligned, it will return -ENOBUFS. Does it an intended behavior?Yeah, I think that's wrong. Does the attached help?
(+cc more people for testing) Will test and feedback later. Thanks, Gao Xiang
quoted hunk ↗ jump to hunk
David ---diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c index bffffedce4a9..7529b40bc95a 100644 --- a/fs/cachefiles/io.c +++ b/fs/cachefiles/io.c@@ -522,16 +522,22 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, bool no_space_allocated_yet) { struct cachefiles_cache *cache = object->volume->cache; - loff_t start = *_start, pos; - size_t len = *_len, down; + unsigned long long start = *_start, pos; + size_t len = *_len; int ret; /* Round to DIO size */ - down = start - round_down(start, PAGE_SIZE); - *_start = start - down; - *_len = round_up(down + len, PAGE_SIZE); - if (down < start || *_len > upper_len) + start = round_down(*_start, PAGE_SIZE); + if (start != *_start) { + kleave(" = -ENOBUFS [down]"); + return -ENOBUFS; + } + if (*_len > upper_len) { + kleave(" = -ENOBUFS [up]"); return -ENOBUFS; + } + + *_len = round_up(len, PAGE_SIZE); /* We need to work out whether there's sufficient disk space to perform * the write - but we can skip that check if we have space already@@ -542,7 +548,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, pos = cachefiles_inject_read_error(); if (pos == 0) - pos = vfs_llseek(file, *_start, SEEK_DATA); + pos = vfs_llseek(file, start, SEEK_DATA); if (pos < 0 && pos >= (loff_t)-MAX_ERRNO) { if (pos == -ENXIO) goto check_space; /* Unallocated tail */@@ -550,7 +556,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, cachefiles_trace_seek_error); return pos; } - if ((u64)pos >= (u64)*_start + *_len) + if (pos >= start + *_len) goto check_space; /* Unallocated region */ /* We have a block that's at least partially filled - if we're low on@@ -563,13 +569,13 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, pos = cachefiles_inject_read_error(); if (pos == 0) - pos = vfs_llseek(file, *_start, SEEK_HOLE); + pos = vfs_llseek(file, start, SEEK_HOLE); if (pos < 0 && pos >= (loff_t)-MAX_ERRNO) { trace_cachefiles_io_error(object, file_inode(file), pos, cachefiles_trace_seek_error); return pos; } - if ((u64)pos >= (u64)*_start + *_len) + if (pos >= start + *_len) return 0; /* Fully allocated */ /* Partially allocated, but insufficient space: cull. */@@ -577,7 +583,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, ret = cachefiles_inject_remove_error(); if (ret == 0) ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, - *_start, *_len); + start, *_len); if (ret < 0) { trace_cachefiles_io_error(object, file_inode(file), ret, cachefiles_trace_fallocate_error);