* [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim
@ 2012-07-30 8:13 ` Tomas Racek
0 siblings, 0 replies; 6+ messages in thread
From: Tomas Racek @ 2012-07-30 8:13 UTC (permalink / raw)
To: linux-xfs
Cc: Alex Elder, open list, Tomas Racek, supporter:XFS FILESYSTEM,
Ben Myers, lczerner
If range.start argument was between ULLONG_MAX - BBSIZE and ULLONG_MAX,
BTOBB macro resulted in overflow which caused start to be set to 0.
Now, invalid argument error is returned instead.
Signed-off-by: Tomas Racek <tracek@redhat.com>
---
fs/xfs/xfs_discard.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index f9c3fe3..0ef7dd4 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -179,6 +179,10 @@ xfs_ioc_trim(
* used by the fstrim application. In the end it really doesn't
* matter as trimming blocks is an advisory interface.
*/
+
+ if (range.start > ULLONG_MAX - BBSIZE)
+ return -XFS_ERROR(EINVAL);
+
start = BTOBB(range.start);
end = start + BTOBBT(range.len) - 1;
minlen = BTOBB(max_t(u64, granularity, range.minlen));
--
1.7.7.6
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim
@ 2012-07-30 8:13 ` Tomas Racek
0 siblings, 0 replies; 6+ messages in thread
From: Tomas Racek @ 2012-07-30 8:13 UTC (permalink / raw)
To: linux-xfs
Cc: lczerner, Tomas Racek, Ben Myers, Alex Elder,
supporter:XFS FILESYSTEM, open list
If range.start argument was between ULLONG_MAX - BBSIZE and ULLONG_MAX,
BTOBB macro resulted in overflow which caused start to be set to 0.
Now, invalid argument error is returned instead.
Signed-off-by: Tomas Racek <tracek@redhat.com>
---
fs/xfs/xfs_discard.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index f9c3fe3..0ef7dd4 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -179,6 +179,10 @@ xfs_ioc_trim(
* used by the fstrim application. In the end it really doesn't
* matter as trimming blocks is an advisory interface.
*/
+
+ if (range.start > ULLONG_MAX - BBSIZE)
+ return -XFS_ERROR(EINVAL);
+
start = BTOBB(range.start);
end = start + BTOBBT(range.len) - 1;
minlen = BTOBB(max_t(u64, granularity, range.minlen));
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim
2012-07-30 8:13 ` Tomas Racek
@ 2012-07-30 21:49 ` Dave Chinner
-1 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2012-07-30 21:49 UTC (permalink / raw)
To: Tomas Racek
Cc: Alex Elder, open list, supporter:XFS FILESYSTEM, linux-xfs,
Ben Myers, lczerner
On Mon, Jul 30, 2012 at 10:13:44AM +0200, Tomas Racek wrote:
> If range.start argument was between ULLONG_MAX - BBSIZE and ULLONG_MAX,
> BTOBB macro resulted in overflow which caused start to be set to 0.
> Now, invalid argument error is returned instead.
>
> Signed-off-by: Tomas Racek <tracek@redhat.com>
> ---
> fs/xfs/xfs_discard.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
> index f9c3fe3..0ef7dd4 100644
> --- a/fs/xfs/xfs_discard.c
> +++ b/fs/xfs/xfs_discard.c
> @@ -179,6 +179,10 @@ xfs_ioc_trim(
> * used by the fstrim application. In the end it really doesn't
> * matter as trimming blocks is an advisory interface.
> */
> +
> + if (range.start > ULLONG_MAX - BBSIZE)
> + return -XFS_ERROR(EINVAL);
> +
There's no point checking for overflow on the range start - what we
need to check is whether it is larger than the size of the
filesystem. We do that after the conversion of range.start to basic
blocks, so that check needs to be promoted to before this. i.e.
if (range.start >= XFS_FSB_TO_B(mp, mp->m_sb.sb_dblocks))
return -XFS_ERROR(EINVAL);
> start = BTOBB(range.start);
> end = start + BTOBBT(range.len) - 1;
> minlen = BTOBB(max_t(u64, granularity, range.minlen));
And that will prevent the overflow in BTOBB() just as effectively...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim
@ 2012-07-30 21:49 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2012-07-30 21:49 UTC (permalink / raw)
To: Tomas Racek
Cc: linux-xfs, lczerner, Ben Myers, Alex Elder,
supporter:XFS FILESYSTEM, open list
On Mon, Jul 30, 2012 at 10:13:44AM +0200, Tomas Racek wrote:
> If range.start argument was between ULLONG_MAX - BBSIZE and ULLONG_MAX,
> BTOBB macro resulted in overflow which caused start to be set to 0.
> Now, invalid argument error is returned instead.
>
> Signed-off-by: Tomas Racek <tracek@redhat.com>
> ---
> fs/xfs/xfs_discard.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
> index f9c3fe3..0ef7dd4 100644
> --- a/fs/xfs/xfs_discard.c
> +++ b/fs/xfs/xfs_discard.c
> @@ -179,6 +179,10 @@ xfs_ioc_trim(
> * used by the fstrim application. In the end it really doesn't
> * matter as trimming blocks is an advisory interface.
> */
> +
> + if (range.start > ULLONG_MAX - BBSIZE)
> + return -XFS_ERROR(EINVAL);
> +
There's no point checking for overflow on the range start - what we
need to check is whether it is larger than the size of the
filesystem. We do that after the conversion of range.start to basic
blocks, so that check needs to be promoted to before this. i.e.
if (range.start >= XFS_FSB_TO_B(mp, mp->m_sb.sb_dblocks))
return -XFS_ERROR(EINVAL);
> start = BTOBB(range.start);
> end = start + BTOBBT(range.len) - 1;
> minlen = BTOBB(max_t(u64, granularity, range.minlen));
And that will prevent the overflow in BTOBB() just as effectively...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim
2012-07-30 21:49 ` Dave Chinner
@ 2012-07-31 10:32 ` Tomas Racek
-1 siblings, 0 replies; 6+ messages in thread
From: Tomas Racek @ 2012-07-31 10:32 UTC (permalink / raw)
To: Dave Chinner
Cc: Alex Elder, open list, supporter:XFS FILESYSTEM, linux-xfs,
Ben Myers, lczerner
> > +
> > + if (range.start > ULLONG_MAX - BBSIZE)
> > + return -XFS_ERROR(EINVAL);
> > +
>
> There's no point checking for overflow on the range start - what we
> need to check is whether it is larger than the size of the
> filesystem. We do that after the conversion of range.start to basic
> blocks, so that check needs to be promoted to before this. i.e.
>
> if (range.start >= XFS_FSB_TO_B(mp, mp->m_sb.sb_dblocks))
> return -XFS_ERROR(EINVAL);
>
> > start = BTOBB(range.start);
> > end = start + BTOBBT(range.len) - 1;
> > minlen = BTOBB(max_t(u64, granularity, range.minlen));
>
> And that will prevent the overflow in BTOBB() just as effectively...
You're right, that's a far better way, I'll change it so.
Thanks!
Tomas
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim
@ 2012-07-31 10:32 ` Tomas Racek
0 siblings, 0 replies; 6+ messages in thread
From: Tomas Racek @ 2012-07-31 10:32 UTC (permalink / raw)
To: Dave Chinner
Cc: Alex Elder, open list, supporter:XFS FILESYSTEM, linux-xfs,
Ben Myers, lczerner
> > +
> > + if (range.start > ULLONG_MAX - BBSIZE)
> > + return -XFS_ERROR(EINVAL);
> > +
>
> There's no point checking for overflow on the range start - what we
> need to check is whether it is larger than the size of the
> filesystem. We do that after the conversion of range.start to basic
> blocks, so that check needs to be promoted to before this. i.e.
>
> if (range.start >= XFS_FSB_TO_B(mp, mp->m_sb.sb_dblocks))
> return -XFS_ERROR(EINVAL);
>
> > start = BTOBB(range.start);
> > end = start + BTOBBT(range.len) - 1;
> > minlen = BTOBB(max_t(u64, granularity, range.minlen));
>
> And that will prevent the overflow in BTOBB() just as effectively...
You're right, that's a far better way, I'll change it so.
Thanks!
Tomas
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-31 10:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-30 8:13 [PATCH 1/1] xfs: check for possible overflow in xfs_ioc_trim Tomas Racek
2012-07-30 8:13 ` Tomas Racek
2012-07-30 21:49 ` Dave Chinner
2012-07-30 21:49 ` Dave Chinner
2012-07-31 10:32 ` Tomas Racek
2012-07-31 10:32 ` Tomas Racek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.