[PATCH] jfs: Fix FITRIM argument handling

[PATCH] jfs: Fix FITRIM argument handling

[Lukas Czerner]

Currently when 'range->start' is beyond the end of file system
nothing is done and that fact is ignored, where in fact we should return
EINVAL. The same problem is when 'range.len' is smaller than file system
block.

Fix this by adding check for such conditions and return EINVAL
appropriately.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/jfs/jfs_discard.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/jfs/jfs_discard.c b/fs/jfs/jfs_discard.c
index 9947563..dfcd503 100644
--- a/fs/jfs/jfs_discard.c
+++ b/fs/jfs/jfs_discard.c
@@ -83,7 +83,7 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
 	struct bmap *bmp = JFS_SBI(ip->i_sb)->bmap;
 	struct super_block *sb = ipbmap->i_sb;
 	int agno, agno_end;
-	s64 start, end, minlen;
+	u64 start, end, minlen;
 	u64 trimmed = 0;
 
 	/**
@@ -93,15 +93,19 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
 	 * minlen:	minimum extent length in Bytes
 	 */
 	start = range->start >> sb->s_blocksize_bits;
-	if (start < 0)
-		start = 0;
 	end = start + (range->len >> sb->s_blocksize_bits) - 1;
-	if (end >= bmp->db_mapsize)
-		end = bmp->db_mapsize - 1;
 	minlen = range->minlen >> sb->s_blocksize_bits;
-	if (minlen <= 0)
+	if (minlen == 0)
 		minlen = 1;
 
+	if (minlen > bmp->db_agsize ||
+	    start >= bmp->db_mapsize ||
+	    range->len < sb->s_blocksize)
+		return -EINVAL;
+
+	if (end >= bmp->db_mapsize)
+		end = bmp->db_mapsize - 1;
+
 	/**
 	 * we trim all ag's within the range
 	 */
-- 
1.7.7.6

Re: [Jfs-discussion] [PATCH] jfs: Fix FITRIM argument handling

[Tino Reichardt]

* Lukas Czerner <lczerner@redhat.com> wrote:
> Currently when 'range->start' is beyond the end of file system
> nothing is done and that fact is ignored, where in fact we should return
> EINVAL. The same problem is when 'range.len' is smaller than file system
> block.
> 
> Fix this by adding check for such conditions and return EINVAL
> appropriately.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Acked-by: Tino Reichardt <milky-kernel@mcmilk.de>

> ---
>  fs/jfs/jfs_discard.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/jfs/jfs_discard.c b/fs/jfs/jfs_discard.c
> index 9947563..dfcd503 100644
> --- a/fs/jfs/jfs_discard.c
> +++ b/fs/jfs/jfs_discard.c
> @@ -83,7 +83,7 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
>  	struct bmap *bmp = JFS_SBI(ip->i_sb)->bmap;
>  	struct super_block *sb = ipbmap->i_sb;
>  	int agno, agno_end;
> -	s64 start, end, minlen;
> +	u64 start, end, minlen;
>  	u64 trimmed = 0;
>  
>  	/**
> @@ -93,15 +93,19 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
>  	 * minlen:	minimum extent length in Bytes
>  	 */
>  	start = range->start >> sb->s_blocksize_bits;
> -	if (start < 0)
> -		start = 0;
>  	end = start + (range->len >> sb->s_blocksize_bits) - 1;
> -	if (end >= bmp->db_mapsize)
> -		end = bmp->db_mapsize - 1;
>  	minlen = range->minlen >> sb->s_blocksize_bits;
> -	if (minlen <= 0)
> +	if (minlen == 0)
>  		minlen = 1;
>  
> +	if (minlen > bmp->db_agsize ||
> +	    start >= bmp->db_mapsize ||
> +	    range->len < sb->s_blocksize)
> +		return -EINVAL;
> +
> +	if (end >= bmp->db_mapsize)
> +		end = bmp->db_mapsize - 1;
> +
>  	/**
>  	 * we trim all ag's within the range
>  	 */
> -- 
> 1.7.7.6
> 
> 
> ------------------------------------------------------------------------------
> Don't let slow site performance ruin your business. Deploy New Relic APM
> Deploy New Relic app performance management and know exactly
> what is happening inside your Ruby, Python, PHP, Java, and .NET app
> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
> http://p.sf.net/sfu/newrelic-dev2dev
> _______________________________________________
> Jfs-discussion mailing list
> Jfs-discussion@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/jfs-discussion

-- 
regards, TR

Re: [PATCH] jfs: Fix FITRIM argument handling

[Dave Kleikamp]

On 10/16/2012 04:38 AM, Lukas Czerner wrote:
> Currently when 'range->start' is beyond the end of file system
> nothing is done and that fact is ignored, where in fact we should return
> EINVAL. The same problem is when 'range.len' is smaller than file system
> block.
> 
> Fix this by adding check for such conditions and return EINVAL
> appropriately.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Looks good. Pushed out to
git://github.com/kleikamp/linux-shaggy.git jfs-next

Should make the next linux-next build.

Thanks,
Shaggy

> ---
>  fs/jfs/jfs_discard.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/jfs/jfs_discard.c b/fs/jfs/jfs_discard.c
> index 9947563..dfcd503 100644
> --- a/fs/jfs/jfs_discard.c
> +++ b/fs/jfs/jfs_discard.c
> @@ -83,7 +83,7 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
>  	struct bmap *bmp = JFS_SBI(ip->i_sb)->bmap;
>  	struct super_block *sb = ipbmap->i_sb;
>  	int agno, agno_end;
> -	s64 start, end, minlen;
> +	u64 start, end, minlen;
>  	u64 trimmed = 0;
>  
>  	/**
> @@ -93,15 +93,19 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
>  	 * minlen:	minimum extent length in Bytes
>  	 */
>  	start = range->start >> sb->s_blocksize_bits;
> -	if (start < 0)
> -		start = 0;
>  	end = start + (range->len >> sb->s_blocksize_bits) - 1;
> -	if (end >= bmp->db_mapsize)
> -		end = bmp->db_mapsize - 1;
>  	minlen = range->minlen >> sb->s_blocksize_bits;
> -	if (minlen <= 0)
> +	if (minlen == 0)
>  		minlen = 1;
>  
> +	if (minlen > bmp->db_agsize ||
> +	    start >= bmp->db_mapsize ||
> +	    range->len < sb->s_blocksize)
> +		return -EINVAL;
> +
> +	if (end >= bmp->db_mapsize)
> +		end = bmp->db_mapsize - 1;
> +
>  	/**
>  	 * we trim all ag's within the range
>  	 */
>