| Message ID | 20240109112029.1572463-6-dhowells@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | netfs, cachefiles: More additional patches | expand |
On 2024/1/9 19:20, David Howells wrote: > In __cachefiles_prepare_write(), the start and pos variables were made > unsigned 64-bit so that the casts in the checking could be got rid of - > which should be fine since absolute file offsets can't be negative, except > that an error code may be obtained from vfs_llseek(), which *would* be > negative. This breaks the error check. > > Fix this for now by reverting pos and start to be signed and putting back > the casts. Unfortunately, the error value checks cannot be replaced with > IS_ERR_VALUE() as long might be 32-bits. > > Fixes: 7097c96411d2 ("cachefiles: Fix __cachefiles_prepare_write()") > Reported-by: Simon Horman <horms@kernel.org> > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202401071152.DbKqMQMu-lkp@intel.com/ > Signed-off-by: David Howells <dhowells@redhat.com> > Reviewed-by: Simon Horman <horms@kernel.org> > cc: Gao Xiang <hsiangkao@linux.alibaba.com> > cc: Yiqun Leng <yqleng@linux.alibaba.com> > cc: Jia Zhu <zhujia.zj@bytedance.com> > cc: Jeff Layton <jlayton@kernel.org> > cc: linux-cachefs@redhat.com > cc: linux-erofs@lists.ozlabs.org > cc: linux-fsdevel@vger.kernel.org > cc: linux-mm@kvack.org It looks good to me, Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> Thanks, Gao Xiang
Tested-by: Jia Zhu <zhujia.zj@bytedance.com> 在 2024/1/9 19:20, David Howells 写道: > In __cachefiles_prepare_write(), the start and pos variables were made > unsigned 64-bit so that the casts in the checking could be got rid of - > which should be fine since absolute file offsets can't be negative, except > that an error code may be obtained from vfs_llseek(), which *would* be > negative. This breaks the error check. > > Fix this for now by reverting pos and start to be signed and putting back > the casts. Unfortunately, the error value checks cannot be replaced with > IS_ERR_VALUE() as long might be 32-bits. > > Fixes: 7097c96411d2 ("cachefiles: Fix __cachefiles_prepare_write()") > Reported-by: Simon Horman <horms@kernel.org> > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202401071152.DbKqMQMu-lkp@intel.com/ > Signed-off-by: David Howells <dhowells@redhat.com> > Reviewed-by: Simon Horman <horms@kernel.org> > cc: Gao Xiang <hsiangkao@linux.alibaba.com> > cc: Yiqun Leng <yqleng@linux.alibaba.com> > cc: Jia Zhu <zhujia.zj@bytedance.com> > cc: Jeff Layton <jlayton@kernel.org> > cc: linux-cachefs@redhat.com > cc: linux-erofs@lists.ozlabs.org > cc: linux-fsdevel@vger.kernel.org > cc: linux-mm@kvack.org > --- > fs/cachefiles/io.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c > index 3eec26967437..9a2cb2868e90 100644 > --- a/fs/cachefiles/io.c > +++ b/fs/cachefiles/io.c > @@ -522,7 +522,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, > bool no_space_allocated_yet) > { > struct cachefiles_cache *cache = object->volume->cache; > - unsigned long long start = *_start, pos; > + loff_t start = *_start, pos; > size_t len = *_len; > int ret; > > @@ -556,7 +556,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, > cachefiles_trace_seek_error); > return pos; > } > - if (pos >= start + *_len) > + if ((u64)pos >= (u64)start + *_len) > goto check_space; /* Unallocated region */ > > /* We have a block that's at least partially filled - if we're low on > @@ -575,7 +575,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, > cachefiles_trace_seek_error); > return pos; > } > - if (pos >= start + *_len) > + if ((u64)pos >= (u64)start + *_len) > return 0; /* Fully allocated */ > > /* Partially allocated, but insufficient space: cull. */ >
diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c index 3eec26967437..9a2cb2868e90 100644 --- a/fs/cachefiles/io.c +++ b/fs/cachefiles/io.c @@ -522,7 +522,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, bool no_space_allocated_yet) { struct cachefiles_cache *cache = object->volume->cache; - unsigned long long start = *_start, pos; + loff_t start = *_start, pos; size_t len = *_len; int ret; @@ -556,7 +556,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, cachefiles_trace_seek_error); return pos; } - if (pos >= start + *_len) + if ((u64)pos >= (u64)start + *_len) goto check_space; /* Unallocated region */ /* We have a block that's at least partially filled - if we're low on @@ -575,7 +575,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object, cachefiles_trace_seek_error); return pos; } - if (pos >= start + *_len) + if ((u64)pos >= (u64)start + *_len) return 0; /* Fully allocated */ /* Partially allocated, but insufficient space: cull. */
In __cachefiles_prepare_write(), the start and pos variables were made unsigned 64-bit so that the casts in the checking could be got rid of - which should be fine since absolute file offsets can't be negative, except that an error code may be obtained from vfs_llseek(), which *would* be negative. This breaks the error check. Fix this for now by reverting pos and start to be signed and putting back the casts. Unfortunately, the error value checks cannot be replaced with IS_ERR_VALUE() as long might be 32-bits. Fixes: 7097c96411d2 ("cachefiles: Fix __cachefiles_prepare_write()") Reported-by: Simon Horman <horms@kernel.org> Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202401071152.DbKqMQMu-lkp@intel.com/ Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Simon Horman <horms@kernel.org> cc: Gao Xiang <hsiangkao@linux.alibaba.com> cc: Yiqun Leng <yqleng@linux.alibaba.com> cc: Jia Zhu <zhujia.zj@bytedance.com> cc: Jeff Layton <jlayton@kernel.org> cc: linux-cachefs@redhat.com cc: linux-erofs@lists.ozlabs.org cc: linux-fsdevel@vger.kernel.org cc: linux-mm@kvack.org --- fs/cachefiles/io.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)