* [PATCH] fuse: BUG_ON's correction in fuse_dev_splice_write() @ 2019-07-23 6:33 Vasily Averin 2019-08-01 11:01 ` Miklos Szeredi 0 siblings, 1 reply; 5+ messages in thread From: Vasily Averin @ 2019-07-23 6:33 UTC (permalink / raw) To: linux-kernel, linux-fsdevel, Miklos Szeredi, Andrey Ryabinin commit 963545357202 ("fuse: reduce allocation size for splice_write") changed size of bufs array, so first BUG_ON should be corrected too. Second BUG_ON become useless, first one also includes the second check: any unsigned nbuf value cannot be less than 0. Fixes: 963545357202 ("fuse: reduce allocation size for splice_write") Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- fs/fuse/dev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index ea8237513dfa..311c7911271c 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -2064,8 +2064,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, struct pipe_buffer *ibuf; struct pipe_buffer *obuf; - BUG_ON(nbuf >= pipe->buffers); - BUG_ON(!pipe->nrbufs); + BUG_ON(nbuf >= pipe->nrbufs); ibuf = &pipe->bufs[pipe->curbuf]; obuf = &bufs[nbuf]; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse: BUG_ON's correction in fuse_dev_splice_write() 2019-07-23 6:33 [PATCH] fuse: BUG_ON's correction in fuse_dev_splice_write() Vasily Averin @ 2019-08-01 11:01 ` Miklos Szeredi 2019-08-19 6:52 ` Vasily Averin 2019-08-19 6:53 ` [PATCH] fuse: BUG_ON " Vasily Averin 0 siblings, 2 replies; 5+ messages in thread From: Miklos Szeredi @ 2019-08-01 11:01 UTC (permalink / raw) To: Vasily Averin; +Cc: linux-kernel, linux-fsdevel, Andrey Ryabinin On Tue, Jul 23, 2019 at 8:33 AM Vasily Averin <vvs@virtuozzo.com> wrote: > > commit 963545357202 ("fuse: reduce allocation size for splice_write") > changed size of bufs array, so first BUG_ON should be corrected too. > Second BUG_ON become useless, first one also includes the second check: > any unsigned nbuf value cannot be less than 0. This patch seems broken: it assumes that pipe->nrbufs doesn't change. Have you actually tested it? Thanks, Miklos ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse: BUG_ON's correction in fuse_dev_splice_write() 2019-08-01 11:01 ` Miklos Szeredi @ 2019-08-19 6:52 ` Vasily Averin 2019-08-19 6:53 ` [PATCH] fuse: BUG_ON " Vasily Averin 1 sibling, 0 replies; 5+ messages in thread From: Vasily Averin @ 2019-08-19 6:52 UTC (permalink / raw) To: Miklos Szeredi; +Cc: linux-kernel, linux-fsdevel, Andrey Ryabinin On 8/1/19 2:01 PM, Miklos Szeredi wrote: > On Tue, Jul 23, 2019 at 8:33 AM Vasily Averin <vvs@virtuozzo.com> wrote: >> >> commit 963545357202 ("fuse: reduce allocation size for splice_write") >> changed size of bufs array, so first BUG_ON should be corrected too. >> Second BUG_ON become useless, first one also includes the second check: >> any unsigned nbuf value cannot be less than 0. > > This patch seems broken: it assumes that pipe->nrbufs doesn't change. > Have you actually tested it? You're right, I've missed it. I've prepared second patch version which fixes first BUG_ON only. checkpatch.pl also advises to replace BUG_ONs to WARN_ONs and 'unsigned' to 'unsigned int' however I'm don't understand what it's better here: - keep all as is, - or merge all changes together, - or do it in separate patches, - or do something else? I believe it makes sense to remove BUG_ONs in separate patch, or may be merge it with current one, but I do not like an idea to fight against bare 'unsigned' in fuse. Could you please comment it? Thank you, Vasily Averin ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] fuse: BUG_ON correction in fuse_dev_splice_write() 2019-08-01 11:01 ` Miklos Szeredi 2019-08-19 6:52 ` Vasily Averin @ 2019-08-19 6:53 ` Vasily Averin 2019-10-03 8:04 ` Miklos Szeredi 1 sibling, 1 reply; 5+ messages in thread From: Vasily Averin @ 2019-08-19 6:53 UTC (permalink / raw) To: linux-kernel, linux-fsdevel, Miklos Szeredi, Andrey Ryabinin commit 963545357202 ("fuse: reduce allocation size for splice_write") changed size of bufs array, so BUG_ON which checks the index of the array shold also be fixed. Fixes: 963545357202 ("fuse: reduce allocation size for splice_write") Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- fs/fuse/dev.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index ea8237513dfa..f4ef6e01642c 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -2029,7 +2029,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, struct file *out, loff_t *ppos, size_t len, unsigned int flags) { - unsigned nbuf; + unsigned nbuf, bsize; unsigned idx; struct pipe_buffer *bufs; struct fuse_copy_state cs; @@ -2043,7 +2043,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, pipe_lock(pipe); - bufs = kvmalloc_array(pipe->nrbufs, sizeof(struct pipe_buffer), + bsize = pipe->nrbufs; + bufs = kvmalloc_array(bsize, sizeof(struct pipe_buffer), GFP_KERNEL); if (!bufs) { pipe_unlock(pipe); @@ -2064,7 +2065,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, struct pipe_buffer *ibuf; struct pipe_buffer *obuf; - BUG_ON(nbuf >= pipe->buffers); + BUG_ON(nbuf >= bsize); BUG_ON(!pipe->nrbufs); ibuf = &pipe->bufs[pipe->curbuf]; obuf = &bufs[nbuf]; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse: BUG_ON correction in fuse_dev_splice_write() 2019-08-19 6:53 ` [PATCH] fuse: BUG_ON " Vasily Averin @ 2019-10-03 8:04 ` Miklos Szeredi 0 siblings, 0 replies; 5+ messages in thread From: Miklos Szeredi @ 2019-10-03 8:04 UTC (permalink / raw) To: Vasily Averin; +Cc: linux-kernel, linux-fsdevel, Andrey Ryabinin On Mon, Aug 19, 2019 at 8:53 AM Vasily Averin <vvs@virtuozzo.com> wrote: > > commit 963545357202 ("fuse: reduce allocation size for splice_write") > changed size of bufs array, so BUG_ON which checks the index of the array > shold also be fixed. > > Fixes: 963545357202 ("fuse: reduce allocation size for splice_write") > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > --- > fs/fuse/dev.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index ea8237513dfa..f4ef6e01642c 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -2029,7 +2029,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, > struct file *out, loff_t *ppos, > size_t len, unsigned int flags) > { > - unsigned nbuf; > + unsigned nbuf, bsize; > unsigned idx; > struct pipe_buffer *bufs; > struct fuse_copy_state cs; > @@ -2043,7 +2043,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, > > pipe_lock(pipe); > > - bufs = kvmalloc_array(pipe->nrbufs, sizeof(struct pipe_buffer), > + bsize = pipe->nrbufs; > + bufs = kvmalloc_array(bsize, sizeof(struct pipe_buffer), > GFP_KERNEL); > if (!bufs) { > pipe_unlock(pipe); > @@ -2064,7 +2065,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, > struct pipe_buffer *ibuf; > struct pipe_buffer *obuf; > > - BUG_ON(nbuf >= pipe->buffers); > + BUG_ON(nbuf >= bsize); > BUG_ON(!pipe->nrbufs); Better turn these into WARN_ON's.. Fixed and applied. Thanks, Miklos ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-03 8:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-23 6:33 [PATCH] fuse: BUG_ON's correction in fuse_dev_splice_write() Vasily Averin 2019-08-01 11:01 ` Miklos Szeredi 2019-08-19 6:52 ` Vasily Averin 2019-08-19 6:53 ` [PATCH] fuse: BUG_ON " Vasily Averin 2019-10-03 8:04 ` Miklos Szeredi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).