Re: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-13 Thread Darrick J. Wong
On Fri, Nov 13, 2015 at 08:47:20AM -0700, Jens Axboe wrote: > On 11/10/2015 11:14 PM, Darrick J. Wong wrote: > >On Wed, Nov 11, 2015 at 05:30:07AM +, Seymour, Shane M wrote: > >>A quick question about this part of the patch: > >> > >>>+ uint64_t end = start + len - 1; > >> > >>>+ if (end >=

Re: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-13 Thread Jens Axboe
On 11/10/2015 11:14 PM, Darrick J. Wong wrote: On Wed, Nov 11, 2015 at 05:30:07AM +, Seymour, Shane M wrote: A quick question about this part of the patch: + uint64_t end = start + len - 1; + if (end >= i_size_read(bdev->bd_inode)) return -EINVAL; +

Re: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-13 Thread Jens Axboe
On 11/10/2015 11:14 PM, Darrick J. Wong wrote: On Wed, Nov 11, 2015 at 05:30:07AM +, Seymour, Shane M wrote: A quick question about this part of the patch: + uint64_t end = start + len - 1; + if (end >= i_size_read(bdev->bd_inode)) return -EINVAL; +

Re: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-13 Thread Darrick J. Wong
On Fri, Nov 13, 2015 at 08:47:20AM -0700, Jens Axboe wrote: > On 11/10/2015 11:14 PM, Darrick J. Wong wrote: > >On Wed, Nov 11, 2015 at 05:30:07AM +, Seymour, Shane M wrote: > >>A quick question about this part of the patch: > >> > >>>+ uint64_t end = start + len - 1; > >> > >>>+ if (end >=

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-12 Thread Seymour, Shane M
> I don't have a device large enough to test for signedness errors, since > passing > huge values for start and len never make it past the i_size_read check. If you have someone trying to bypass your sanity checks then if start=18446744073709551104 and len=1024 the result of adding them

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-12 Thread Seymour, Shane M
> I don't have a device large enough to test for signedness errors, since > passing > huge values for start and len never make it past the i_size_read check. If you have someone trying to bypass your sanity checks then if start=18446744073709551104 and len=1024 the result of adding them

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-11 Thread Seymour, Shane M
> which would make the other checks I suggested to ensure that neither start > or end were more than (uint64_t)LLONG_MAX unnecessary. My apologies I was wrong about what I said above - after thinking about it for longer you still need to make sure that at least len is not greater than

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-11 Thread Seymour, Shane M
> I don't have a device large enough to test for signedness errors, since > passing > huge values for start and len never make it past the i_size_read check. > However, I do see that I forgot to check the padding values, so I'll update > that. Then do you want to at least consider converting

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-11 Thread Seymour, Shane M
> I don't have a device large enough to test for signedness errors, since > passing > huge values for start and len never make it past the i_size_read check. > However, I do see that I forgot to check the padding values, so I'll update > that. Then do you want to at least consider converting

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-11 Thread Seymour, Shane M
> which would make the other checks I suggested to ensure that neither start > or end were more than (uint64_t)LLONG_MAX unnecessary. My apologies I was wrong about what I said above - after thinking about it for longer you still need to make sure that at least len is not greater than

Re: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-10 Thread Darrick J. Wong
On Wed, Nov 11, 2015 at 05:30:07AM +, Seymour, Shane M wrote: > A quick question about this part of the patch: > > > + uint64_t end = start + len - 1; > > > + if (end >= i_size_read(bdev->bd_inode)) > return -EINVAL; > > > + /* Invalidate the page cache, including dirty

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-10 Thread Seymour, Shane M
A quick question about this part of the patch: > + uint64_t end = start + len - 1; > + if (end >= i_size_read(bdev->bd_inode)) return -EINVAL; > + /* Invalidate the page cache, including dirty pages */ > + mapping = bdev->bd_inode->i_mapping; > +

Re: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-10 Thread Darrick J. Wong
On Wed, Nov 11, 2015 at 05:30:07AM +, Seymour, Shane M wrote: > A quick question about this part of the patch: > > > + uint64_t end = start + len - 1; > > > + if (end >= i_size_read(bdev->bd_inode)) > return -EINVAL; > > > + /* Invalidate the page cache, including dirty

RE: [PATCH] block: create ioctl to discard-or-zeroout a range of blocks

2015-11-10 Thread Seymour, Shane M
A quick question about this part of the patch: > + uint64_t end = start + len - 1; > + if (end >= i_size_read(bdev->bd_inode)) return -EINVAL; > + /* Invalidate the page cache, including dirty pages */ > + mapping = bdev->bd_inode->i_mapping; > +