| Message ID | 20200429140341.13294-2-maco@android.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a new LOOP_CONFIGURE ioctl | expand |
On Wed, Apr 29, 2020 at 04:03:32PM +0200, Martijn Coenen wrote: > Ensuring we don't truncate loff_t when casting to sector_t is done in > multiple places; factor it out. > > Signed-off-by: Martijn Coenen <maco@android.com> > --- > drivers/block/loop.c | 25 ++++++++++++++++++++----- > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index f1754262fc94..396b8bd4d75c 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -228,15 +228,30 @@ static void __loop_update_dio(struct loop_device *lo, bool dio) > blk_mq_unfreeze_queue(lo->lo_queue); > } > > +/** > + * loop_validate_size() - validates that the passed in size fits in a sector_t > + * @size: size to validate > + */ > +static int > +loop_validate_size(loff_t size) > +{ > + if ((loff_t)(sector_t)size != size) > + return -EFBIG; > + > + return 0; > +} > + Now sector_t has been switched to u64 unconditionally, do we still need such validation? Thanks, Ming
Hi Ming, On Wed, Apr 29, 2020 at 4:12 PM Ming Lei <ming.lei@redhat.com> wrote: > Now sector_t has been switched to u64 unconditionally, do we still need such > validation? I think you're right; I hadn't seen that change, but truncating because of sector_t shouldn't be an issue anymore. I wondered if we could actually have a smaller loff_t, but looks like that is 'long long', which should always be 8 bytes as well. I might send this as a separate patch, I don't want to drag this series on for too long. Thanks, Martijn > > > Thanks, > Ming > >
On Wed, Apr 29, 2020 at 10:12:29PM +0800, Ming Lei wrote: > > +static int > > +loop_validate_size(loff_t size) > > +{ > > + if ((loff_t)(sector_t)size != size) > > + return -EFBIG; > > + > > + return 0; > > +} > > + > > Now sector_t has been switched to u64 unconditionally, do we still need such > validation? Oops, completely forgot about that. Yes, we can just kill the checks.
diff --git a/drivers/block/loop.c b/drivers/block/loop.c index f1754262fc94..396b8bd4d75c 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -228,15 +228,30 @@ static void __loop_update_dio(struct loop_device *lo, bool dio) blk_mq_unfreeze_queue(lo->lo_queue); } +/** + * loop_validate_size() - validates that the passed in size fits in a sector_t + * @size: size to validate + */ +static int +loop_validate_size(loff_t size) +{ + if ((loff_t)(sector_t)size != size) + return -EFBIG; + + return 0; +} + static int figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit) { + int err; loff_t size = get_size(offset, sizelimit, lo->lo_backing_file); - sector_t x = (sector_t)size; struct block_device *bdev = lo->lo_device; - if (unlikely((loff_t)x != size)) - return -EFBIG; + err = loop_validate_size(size); + if (err) + return err; + if (lo->lo_offset != offset) lo->lo_offset = offset; if (lo->lo_sizelimit != sizelimit) @@ -1003,9 +1018,9 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, !file->f_op->write_iter) lo_flags |= LO_FLAGS_READ_ONLY; - error = -EFBIG; size = get_loop_size(lo, file); - if ((loff_t)(sector_t)size != size) + error = loop_validate_size(size); + if (error) goto out_unlock; error = loop_prepare_queue(lo); if (error)
Ensuring we don't truncate loff_t when casting to sector_t is done in multiple places; factor it out. Signed-off-by: Martijn Coenen <maco@android.com> --- drivers/block/loop.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)