diff mbox series

[V2,1/2] target: fix selinux error when systemd-modules loads the target module

Message ID 20240215143944.847184-2-mlombard@redhat.com (mailing list archive)
State Accepted
Headers show
Series Fix SELinux denials against target driver | expand

Commit Message

Maurizio Lombardi Feb. 15, 2024, 2:39 p.m. UTC
If the systemd-modules service loads the target module, the credentials
of that userspace process will be used to validate the access to the
target db directory.
selinux will prevent it, reporting an error like the following:

kernel: audit: type=1400 audit(1676301082.205:4): avc: denied  { read }
for  pid=1020 comm="systemd-modules" name="target" dev="dm-3"
ino=4657583 scontext=system_u:system_r:systemd_modules_load_t:s0
tcontext=system_u:object_r:targetd_etc_rw_t:s0 tclass=dir permissive=0

Fix the error by using the kernel credentials to access the db directory

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---

v2: call target_xcopy_release_pt() to destroy the xcopy_wq in case of failure

 drivers/target/target_core_configfs.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Mike Christie Feb. 15, 2024, 4:44 p.m. UTC | #1
On 2/15/24 8:39 AM, Maurizio Lombardi wrote:
> If the systemd-modules service loads the target module, the credentials
> of that userspace process will be used to validate the access to the
> target db directory.
> selinux will prevent it, reporting an error like the following:
> 
> kernel: audit: type=1400 audit(1676301082.205:4): avc: denied  { read }
> for  pid=1020 comm="systemd-modules" name="target" dev="dm-3"
> ino=4657583 scontext=system_u:system_r:systemd_modules_load_t:s0
> tcontext=system_u:object_r:targetd_etc_rw_t:s0 tclass=dir permissive=0
> 
> Fix the error by using the kernel credentials to access the db directory
> 

Do you need something similar for the pr related dirs/files or how does
that work but not this?
Maurizio Lombardi Feb. 15, 2024, 5:07 p.m. UTC | #2
čt 15. 2. 2024 v 17:44 odesílatel <michael.christie@oracle.com> napsal:
> Do you need something similar for the pr related dirs/files or how does
> that work but not this?
>

I think that in that case it won't be necessary because the pr code is executed
by a kernel thread that calls the execute_cmd() callback, not by a
user process in kernel context,
but I will try and eventually I will report back the findings

Maurizio
Mike Christie Feb. 15, 2024, 9:06 p.m. UTC | #3
On 2/15/24 11:07 AM, Maurizio Lombardi wrote:
> čt 15. 2. 2024 v 17:44 odesílatel <michael.christie@oracle.com> napsal:
>> Do you need something similar for the pr related dirs/files or how does
>> that work but not this?
>>
> 
> I think that in that case it won't be necessary because the pr code is executed
> by a kernel thread that calls the execute_cmd() callback, not by a
> user process in kernel context,
> but I will try and eventually I will report back the findings
> 

Ignore my comment. I was thinking of something completely different and
figured out I was wrong when I looked into it.

The target parts look ok:

Reviewed-by: Mike Christie <michael.christie@oracle.com>

I have no idea about the creds part. If the patch:

commit 581dd69830341d299b0c097fc366097ab497d679
Author: Thiébaud Weksteen <tweek@google.com>
Date:   Mon May 2 10:49:52 2022 +1000

    firmware_loader: use kernel credentials when reading firmware

you referenced is the correct way to do it, then it looks ok.
diff mbox series

Patch

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index a5f58988130a..3f3dd0c0ce8c 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -3656,6 +3656,8 @@  static int __init target_core_init_configfs(void)
 {
 	struct configfs_subsystem *subsys = &target_core_fabrics;
 	struct t10_alua_lu_gp *lu_gp;
+	struct cred *kern_cred;
+	const struct cred *old_cred;
 	int ret;
 
 	pr_debug("TARGET_CORE[0]: Loading Generic Kernel Storage"
@@ -3732,11 +3734,21 @@  static int __init target_core_init_configfs(void)
 	if (ret < 0)
 		goto out;
 
+	/* We use the kernel credentials to access the target directory */
+	kern_cred = prepare_kernel_cred(&init_task);
+	if (!kern_cred) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	old_cred = override_creds(kern_cred);
 	target_init_dbroot();
+	revert_creds(old_cred);
+	put_cred(kern_cred);
 
 	return 0;
 
 out:
+	target_xcopy_release_pt();
 	configfs_unregister_subsystem(subsys);
 	core_dev_release_virtual_lun0();
 	rd_module_exit();