integrity: silence warning when CONFIG_SECURITYFS is not enabled

PATCH Superseded Sudeep Holla 3 participants 9 messages Jun 4, 2018 – Jun 13, 2018

Subsystems (3)

Security (LSM) Integrity measurement architecture (IMA) Extended Verification Module (EVM)

lore

Share & tools

lore

Sudeep Holla

author

2018-06-04 14:05 UTC #0 lore raw

PATCH 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"

However, if the feature is disabled, it can't be warning and hence we need to silence the error.

Cc James Morris <[email protected]>

Cc "Serge E. Hallyn" <[email protected]>

Cc Matthew Garrett <[email protected]>

Signed-off-by Sudeep Holla <[email protected]>

---
 security/integrity/iint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

security/integrity/iint.c +1 -1

index 149faa81f6f0..8082491876f9 100644
@@ -218,7 +218,7 @@ void __init integrity_load_keys(void)
 static int __init integrity_fs_init(void)
 {
 	integrity_dir = securityfs_create_dir("integrity", NULL);
-	if (IS_ERR(integrity_dir)) {
+	if (IS_ENABLED(CONFIG_SECURITYFS) && IS_ERR(integrity_dir)) {
 		pr_err("Unable to create integrity sysfs dir: %ld\n",
 		       PTR_ERR(integrity_dir));
 		integrity_dir = NULL;

Signature · 6 lines

2.7.4

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

Matthew Garrett

2018-06-04 17:44 UTC #1 lore raw

PATCH 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

Quoted 6 lines · Sudeep Holla

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"
However, if the feature is disabled, it can't be warning and hence we need to silence the error.

I think it'd be preferable to check whether it's ENODEV rather than doing IS_ENABLED. We should also reset integrity_dir to NULL rather than leaving it as an error - it gets passed into various other functions, and while those should all also just be returning errors it'd be unfortunate if one attempted to dereference it.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sudeep Holla

author

2018-06-05 10:25 UTC #2 lore raw

PATCH v2 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

In reply to Sudeep Holla

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"

However, if the feature is disabled, it can't be warning and hence we need to silence the error. This patch checks for the error -ENODEV which is returned when CONFIG_SECURITYFS is disabled to stop the error being thrown.

Cc James Morris <[email protected]>

Cc "Serge E. Hallyn" <[email protected]>

Cc Matthew Garrett <[email protected]>

Signed-off-by Sudeep Holla <[email protected]>

---
 security/integrity/iint.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

v1->v2:
- Check for -ENODEV rather than IS_ENABLED(..) as suggested by Matthew Garrett

security/integrity/iint.c +6 -3

index 149faa81f6f0..7051ea4a8161 100644
@@ -219,10 +219,13 @@ static int __init integrity_fs_init(void)
 {
 	integrity_dir = securityfs_create_dir("integrity", NULL);
 	if (IS_ERR(integrity_dir)) {
-		pr_err("Unable to create integrity sysfs dir: %ld\n",
-		       PTR_ERR(integrity_dir));
+		int ret = PTR_ERR(integrity_dir);
+
+		if (ret != -ENODEV)
+			pr_err("Unable to create integrity sysfs dir: %ld\n",
+			       ret;
 		integrity_dir = NULL;
-		return PTR_ERR(integrity_dir);
+		return ret;
 	}
 
 	return 0;

--
2.7.4

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

Mimi Zohar

2018-06-05 14:49 UTC #3 lore raw

PATCH v2 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

Quoted 8 lines · Sudeep Holla

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"
However, if the feature is disabled, it can't be warning and hence we need to silence the error. This patch checks for the error -ENODEV which is returned when CONFIG_SECURITYFS is disabled to stop the error being thrown.

Both IMA and EVM require securityfs, at least for the time being. ?Under what circumstances would integrity and not securityfs be enabled. ?Is this a Kconfig issue?

Mimi

Quoted 37 lines

Cc James Morris <[email protected]>

Cc "Serge E. Hallyn" <[email protected]>

Cc Matthew Garrett <[email protected]>

Signed-off-by Sudeep Holla <[email protected]>

---
 security/integrity/iint.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

v1->v2:
- Check for -ENODEV rather than IS_ENABLED(..) as suggested by Matthew Garrett

security/integrity/iint.c +6 -4

index 149faa81f6f0..7051ea4a8161 100644
@@ -219,10 +219,13 @@ static int __init integrity_fs_init(void)
 {
 	integrity_dir = securityfs_create_dir("integrity", NULL);
 	if (IS_ERR(integrity_dir)) {
-		pr_err("Unable to create integrity sysfs dir: %ld\n",
-		       PTR_ERR(integrity_dir));
+		int ret = PTR_ERR(integrity_dir);
+
+		if (ret != -ENODEV)
+			pr_err("Unable to create integrity sysfs dir: %ld\n",
+			       ret;
 		integrity_dir = NULL;
-		return PTR_ERR(integrity_dir);
+		return ret;
 	}
 
 	return 0;
--

2.7.4

Quoted 5 lines

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

Sudeep Holla

author

2018-06-06 09:23 UTC #4 lore raw

PATCH v2 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

Quoted 5 lines · Mimi Zohar

Quoted 4 lines · Sudeep Holla
When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"

Quoted 9 lines

Quoted 4 lines
However, if the feature is disabled, it can't be warning and hence we need to silence the error. This patch checks for the error -ENODEV which is returned when CONFIG_SECURITYFS is disabled to stop the error being thrown.
Both IMA and EVM require securityfs, at least for the time being. ?Under what circumstances would integrity and not securityfs be enabled. ?Is this a Kconfig issue?

Could be, looks like it's not enforced and hence I have ended up with a config that has CONFIG_SECURITYFS disabled. I have bot IMA and EVM disabled too. However CONFIG_INTEGRITY is enabled.

Signature · 6 lines

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mimi Zohar

2018-06-06 13:24 UTC #5 lore raw

PATCH v2 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

In reply to Sudeep Holla

Quoted 26 lines · Sudeep Holla

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"
However, if the feature is disabled, it can't be warning and hence we need to silence the error. This patch checks for the error -ENODEV which is returned when CONFIG_SECURITYFS is disabled to stop the error being thrown.
Cc James Morris <[email protected]>
Cc "Serge E. Hallyn" <[email protected]>
Cc Matthew Garrett <[email protected]>
Signed-off-by Sudeep Holla <[email protected]>
---
 security/integrity/iint.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
v1->v2:
- Check for -ENODEV rather than IS_ENABLED(..) as suggested by Matthew Garrett
security/integrity/iint.c +0 -0
index 149faa81f6f0..7051ea4a8161 100644

security/integrity/iint.c static int __init integrity_fs_init(void) · line 226

Quoted 10 lines

security/integrity/iint.c +5 -2

@@ -219,10 +219,13 @@ static int __init integrity_fs_init(void)
 {
 	integrity_dir = securityfs_create_dir("integrity", NULL);
 	if (IS_ERR(integrity_dir)) {
-		pr_err("Unable to create integrity sysfs dir: %ld\n",
-		       PTR_ERR(integrity_dir));
+		int ret = PTR_ERR(integrity_dir);
+
+		if (ret != -ENODEV)
+			pr_err("Unable to create integrity sysfs dir: %ld\n",
+			       ret;

Can we replace "sysfs" to "securityfs" at the same time?

Quoted 9 lines

security/integrity/iint.c +1 -2

 		integrity_dir = NULL;
-		return PTR_ERR(integrity_dir);
+		return ret;
 	}
 
 	return 0;
--

2.7.4

Quoted 5 lines

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

Matthew Garrett

2018-06-06 21:07 UTC #6 lore raw

PATCH v2 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

In reply to Sudeep Holla

Quoted 10 lines · Sudeep Holla

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"
However, if the feature is disabled, it can't be warning and hence we need to silence the error. This patch checks for the error -ENODEV which is returned when CONFIG_SECURITYFS is disabled to stop the error being thrown.

Quoted 4 lines

Cc James Morris <[email protected]>
Cc "Serge E. Hallyn" <[email protected]>
Cc Matthew Garrett <[email protected]>
Signed-off-by Sudeep Holla <[email protected]>

Acked-by Matthew Garrett <[email protected]>

Sudeep Holla

author

2018-06-13 15:00 UTC #7 lore raw

PATCH v3 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

In reply to Sudeep Holla

When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error:
"Unable to create integrity sysfs dir: -19"

Cc Mimi Zohar <[email protected]>

Cc James Morris <[email protected]>

Cc "Serge E. Hallyn" <[email protected]>

Cc Matthew Garrett <[email protected]>

Acked-by Matthew Garrett <[email protected]>

Signed-off-by Sudeep Holla <[email protected]>

---
 security/integrity/iint.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Hi Mimi Zohar,

Extremely sorry for the silly mistake. Somehow my aarch64 toolchain doesn't complain about this and I failed to notice though it's so obvious.

Regards,
Sudeep

v2->v3:
- Fix the format specifier for pr_err(%d instead of %ld)
- Replace "sysfs" to "securityfs" in the dmesg as suggested by Mimi Zohar
v1->v2:
- Check for -ENODEV rather than IS_ENABLED(..) as suggested by Matthew Garrett

security/integrity/iint.c +6 -3

index 149faa81f6f0..ba605714aac4 100644
@@ -219,10 +219,13 @@ static int __init integrity_fs_init(void)
 {
 	integrity_dir = securityfs_create_dir("integrity", NULL);
 	if (IS_ERR(integrity_dir)) {
-		pr_err("Unable to create integrity sysfs dir: %ld\n",
-		       PTR_ERR(integrity_dir));
+		int ret = PTR_ERR(integrity_dir);
+
+		if (ret != -ENODEV)
+			pr_err("Unable to create integrity securityfs dir: %d\n",
+			       ret);
 		integrity_dir = NULL;
-		return PTR_ERR(integrity_dir);
+		return ret;
 	}
 
 	return 0;

Signature · 6 lines

2.7.4

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

Mimi Zohar

2018-06-13 22:13 UTC #8 lore raw

PATCH v3 1/1 integrity: silence warning when CONFIG_SECURITYFS is not enabled

Quoted 5 lines · Sudeep Holla

Hi Mimi Zohar,
Extremely sorry for the silly mistake. Somehow my aarch64 toolchain doesn't complain about this and I failed to notice though it's so obvious.

Thanks for the updated patch. It's now in next-integrity-queued branch.

Mimi