[PATCH] btrfs: verify max_inline mount parameter

[PATCH] btrfs: verify max_inline mount parameter

[Anand Jain]

We aren't verifying the parameter passed to the max_inline mount option,
so we won't report and fail the mount if a junk value is specified for
example, -o max_inline=abc.
This patch converts the max_inline option to %d and checks if it's a
number >= 0.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 383be3609cc9..485e12657bc4 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -344,7 +344,7 @@ static const match_table_t tokens = {
 	{Opt_datacow, "datacow"},
 	{Opt_nobarrier, "nobarrier"},
 	{Opt_barrier, "barrier"},
-	{Opt_max_inline, "max_inline=%s"},
+	{Opt_max_inline, "max_inline=%d"},
 	{Opt_alloc_start, "alloc_start=%s"},
 	{Opt_thread_pool, "thread_pool=%d"},
 	{Opt_compress, "compress"},
@@ -407,7 +407,7 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 			unsigned long new_flags)
 {
 	substring_t args[MAX_OPT_ARGS];
-	char *p, *num;
+	char *p;
 	u64 cache_gen;
 	int intarg;
 	int ret = 0;
@@ -604,22 +604,18 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 			}
 			break;
 		case Opt_max_inline:
-			num = match_strdup(&args[0]);
-			if (num) {
-				info->max_inline = memparse(num, NULL);
-				kfree(num);
-
-				if (info->max_inline) {
-					info->max_inline = min_t(u64,
-						info->max_inline,
-						info->sectorsize);
-				}
-				btrfs_info(info, "max_inline at %llu",
-					   info->max_inline);
-			} else {
-				ret = -ENOMEM;
+			ret = match_int(&args[0], &intarg);
+			if (ret) {
+				ret = -EINVAL;
+				goto out;
+			} else if (intarg < 0) {
+				btrfs_err(info, "invalid max_inline=%d\n", intarg);
+				ret = -EINVAL;
 				goto out;
 			}
+			info->max_inline = min_t(u64, intarg, info->sectorsize);
+			btrfs_info(info, "max_inline at %llu",
+				   info->max_inline);
 			break;
 		case Opt_alloc_start:
 			btrfs_info(info,
-- 
2.15.0

Re: [PATCH] btrfs: verify max_inline mount parameter

[David Sterba]

On Mon, Feb 12, 2018 at 11:35:46PM +0800, Anand Jain wrote:
> We aren't verifying the parameter passed to the max_inline mount option,
> so we won't report and fail the mount if a junk value is specified for
> example, -o max_inline=abc.
> This patch converts the max_inline option to %d and checks if it's a
> number >= 0.

Oh right, the parser can verify that for us. As the %d gets stored to
int, there's no reason to store fs_info::max_inline as u64.

Looking at the parser capabilities, it accepts %u and matches an
unsigned type, which we use for all our options. It would be good to
unify that too.

Please update this patch and possibly send more cleaning up the rest of
the match strings, and the max_inline type. Thanks.

Re: [PATCH] btrfs: verify max_inline mount parameter

[Anand Jain]

On 02/13/2018 01:13 AM, David Sterba wrote:
> On Mon, Feb 12, 2018 at 11:35:46PM +0800, Anand Jain wrote:
>> We aren't verifying the parameter passed to the max_inline mount option,
>> so we won't report and fail the mount if a junk value is specified for
>> example, -o max_inline=abc.
>> This patch converts the max_inline option to %d and checks if it's a
>> number >= 0.
> 
> Oh right, the parser can verify that for us. As the %d gets stored to
> int, there's no reason to store fs_info::max_inline as u64.

  fixed this in the patch set sent out.

> Looking at the parser capabilities, it accepts %u and matches an
> unsigned type, which we use for all our options. It would be good to
> unify that too.

  fixed this too.

> Please update this patch and possibly send more cleaning up the rest of
> the match strings, and the max_inline type. Thanks.

  Sure. will do.

Thanks, Anand

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>