Skip to contentSkip to conversation

f2fs: fix return value of f2fs_set_qf_name and parse_options

PATCH Superseded Tiezhu Yang 2 participants 2 messages Aug 11, 2018 – Aug 13, 2018 F2FS file system

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 f2fs: fix return value of f2fs_set_qf_name and parse_options

match_strdup() returns NULL when kmalloc failed, so the caller function f2fs_set_qf_name() should return -ENOMEM in this case; match_int() returns -ENOMEM, -EINVAL, or -ERANGE on failure, so the caller function parse_options() should not always return -EINVAL in this case.

---
 fs/f2fs/super.c | 60 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 19 deletions(-)
fs/f2fs/super.c +41 -19
index 53d70b6..bf6505e 100644
@@ -269,7 +269,7 @@ static int f2fs_set_qf_name(struct super_block *sb, int qtype,
 	if (!qname) {
 		f2fs_msg(sb, KERN_ERR,
 			"Not enough memory for storing quotafile name");
-		return -EINVAL;
+		return -ENOMEM;
 	}
 	if (F2FS_OPTION(sbi).s_qf_names[qtype]) {
 		if (strcmp(F2FS_OPTION(sbi).s_qf_names[qtype], qname) == 0)
@@ -366,9 +366,7 @@ static int parse_options(struct super_block *sb, char *options)
 	int arg = 0;
 	kuid_t uid;
 	kgid_t gid;
-#ifdef CONFIG_QUOTA
 	int ret;
-#endif
 
 	if (!options)
 		return 0;
@@ -452,8 +450,11 @@ static int parse_options(struct super_block *sb, char *options)
 			clear_opt(sbi, INLINE_XATTR);
 			break;
 		case Opt_inline_xattr_size:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			set_opt(sbi, INLINE_XATTR_SIZE);
 			F2FS_OPTION(sbi).inline_xattr_size = arg;
 			break;
@@ -491,8 +492,11 @@ static int parse_options(struct super_block *sb, char *options)
 			break;
 #endif
 		case Opt_active_logs:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			if (arg != 2 && arg != 4 && arg != NR_CURSEG_TYPE)
 				return -EINVAL;
 			F2FS_OPTION(sbi).active_logs = arg;
@@ -534,8 +538,11 @@ static int parse_options(struct super_block *sb, char *options)
 			set_opt(sbi, DATA_FLUSH);
 			break;
 		case Opt_reserve_root:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			if (test_opt(sbi, RESERVE_ROOT)) {
 				f2fs_msg(sb, KERN_INFO,
 					"Preserve previous reserve_root=%u",
@@ -546,8 +553,11 @@ static int parse_options(struct super_block *sb, char *options)
 			}
 			break;
 		case Opt_resuid:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			uid = make_kuid(current_user_ns(), arg);
 			if (!uid_valid(uid)) {
 				f2fs_msg(sb, KERN_ERR,
@@ -557,8 +567,11 @@ static int parse_options(struct super_block *sb, char *options)
 			F2FS_OPTION(sbi).s_resuid = uid;
 			break;
 		case Opt_resgid:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			gid = make_kgid(current_user_ns(), arg);
 			if (!gid_valid(gid)) {
 				f2fs_msg(sb, KERN_ERR,
@@ -592,8 +605,11 @@ static int parse_options(struct super_block *sb, char *options)
 			kfree(name);
 			break;
 		case Opt_io_size_bits:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			if (arg > __ilog2_u32(BIO_MAX_PAGES)) {
 				f2fs_msg(sb, KERN_WARNING,
 					"Not support %d, larger than %d",
@@ -603,8 +619,11 @@ static int parse_options(struct super_block *sb, char *options)
 			F2FS_OPTION(sbi).write_io_size_bits = arg;
 			break;
 		case Opt_fault_injection:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 #ifdef CONFIG_F2FS_FAULT_INJECTION
 			f2fs_build_fault_attr(sbi, arg, F2FS_ALL_FAULT_TYPE);
 			set_opt(sbi, FAULT_INJECTION);
@@ -614,8 +633,11 @@ static int parse_options(struct super_block *sb, char *options)
 #endif
 			break;
 		case Opt_fault_type:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 #ifdef CONFIG_F2FS_FAULT_INJECTION
 			f2fs_build_fault_attr(sbi, 0, arg);
 			set_opt(sbi, FAULT_INJECTION);
Signature · 4 lines
1.8.3.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
#1 lore raw
Quoted 35 lines · Tiezhu Yang

match_strdup() returns NULL when kmalloc failed, so the caller function f2fs_set_qf_name() should return -ENOMEM in this case; match_int() returns -ENOMEM, -EINVAL, or -ERANGE on failure, so the caller function parse_options() should not always return -EINVAL in this case.

---
 fs/f2fs/super.c | 60 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 19 deletions(-)
fs/f2fs/super.c +1 -3
index 53d70b6..bf6505e 100644
@@ -269,7 +269,7 @@ static int f2fs_set_qf_name(struct super_block *sb, int qtype,
 	if (!qname) {
 		f2fs_msg(sb, KERN_ERR,
 			"Not enough memory for storing quotafile name");
-		return -EINVAL;
+		return -ENOMEM;
 	}
 	if (F2FS_OPTION(sbi).s_qf_names[qtype]) {
 		if (strcmp(F2FS_OPTION(sbi).s_qf_names[qtype], qname) == 0)
@@ -366,9 +366,7 @@ static int parse_options(struct super_block *sb, char *options)
 	int arg = 0;
 	kuid_t uid;
 	kgid_t gid;
-#ifdef CONFIG_QUOTA
 	int ret;
-#endif
 
 	if (!options)
 		return 0;

fs/f2fs/super.c static int parse_options(struct super_block *sb, char *options) · line 456

Quoted 9 lines
fs/f2fs/super.c +4 -2
@@ -452,8 +450,11 @@ static int parse_options(struct super_block *sb, char *options)
 			clear_opt(sbi, INLINE_XATTR);
 			break;
 		case Opt_inline_xattr_size:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;> +			}

if (args->from)

	return -EINVAL;
ret = match_init();
if (ret)
	return ret;

The same as below.

Thanks,

Quoted 92 lines
fs/f2fs/super.c +30 -12
 			set_opt(sbi, INLINE_XATTR_SIZE);
 			F2FS_OPTION(sbi).inline_xattr_size = arg;
 			break;
@@ -491,8 +492,11 @@ static int parse_options(struct super_block *sb, char *options)
 			break;
 #endif
 		case Opt_active_logs:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			if (arg != 2 && arg != 4 && arg != NR_CURSEG_TYPE)
 				return -EINVAL;
 			F2FS_OPTION(sbi).active_logs = arg;
@@ -534,8 +538,11 @@ static int parse_options(struct super_block *sb, char *options)
 			set_opt(sbi, DATA_FLUSH);
 			break;
 		case Opt_reserve_root:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			if (test_opt(sbi, RESERVE_ROOT)) {
 				f2fs_msg(sb, KERN_INFO,
 					"Preserve previous reserve_root=%u",
@@ -546,8 +553,11 @@ static int parse_options(struct super_block *sb, char *options)
 			}
 			break;
 		case Opt_resuid:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			uid = make_kuid(current_user_ns(), arg);
 			if (!uid_valid(uid)) {
 				f2fs_msg(sb, KERN_ERR,
@@ -557,8 +567,11 @@ static int parse_options(struct super_block *sb, char *options)
 			F2FS_OPTION(sbi).s_resuid = uid;
 			break;
 		case Opt_resgid:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			gid = make_kgid(current_user_ns(), arg);
 			if (!gid_valid(gid)) {
 				f2fs_msg(sb, KERN_ERR,
@@ -592,8 +605,11 @@ static int parse_options(struct super_block *sb, char *options)
 			kfree(name);
 			break;
 		case Opt_io_size_bits:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 			if (arg > __ilog2_u32(BIO_MAX_PAGES)) {
 				f2fs_msg(sb, KERN_WARNING,
 					"Not support %d, larger than %d",
@@ -603,8 +619,11 @@ static int parse_options(struct super_block *sb, char *options)
 			F2FS_OPTION(sbi).write_io_size_bits = arg;
 			break;
 		case Opt_fault_injection:
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 #ifdef CONFIG_F2FS_FAULT_INJECTION
 			f2fs_build_fault_attr(sbi, arg, F2FS_ALL_FAULT_TYPE);
 			set_opt(sbi, FAULT_INJECTION);
@@ -614,8 +633,11 @@ static int parse_options(struct super_block *sb, char *options)
 #endif
 			break;
 		case Opt_fault_type:

fs/f2fs/super.c static int parse_options(struct super_block *sb, char *options) · line 643

Quoted 10 lines
fs/f2fs/super.c +5 -2
-			if (args->from && match_int(args, &arg))
-				return -EINVAL;
+			if (args->from) {
+				ret = match_int(args, &arg);
+				if (ret)
+					return ret;
+			}
 #ifdef CONFIG_F2FS_FAULT_INJECTION
 			f2fs_build_fault_attr(sbi, 0, arg);
 			set_opt(sbi, FAULT_INJECTION);

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot