Skip to contentSkip to conversation

tomoyo: fix handling of path{1,2}.parent.* conditions

PATCH In review Christian Brauner 2 participants 2 messages Jun 7, 2022

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tomoyo: fix handling of path{1,2}.parent.* conditions

When path conditions are specified tomoyo tries to retrieve information about the parent dentry. It currently assumes that the parent dentry is always reachable from the child dentry's mount. This assumption is wrong when bind-mounts are in play:

mkdir /foo
touch /foo/file1

mkdir /bar
touch /bar/file2

mount --bind /bar/file2 /foo/file1

file read /foo/file1 path1.parent.uid=12

Tomoyo will now call dget_parent(file1). This will yield "bar". But "bar" isn't reachable from the bind-mount of "file1". Handle this case by ensuring that the parent dentry is actually reachable from the child dentry's mount and if not skip it.

Fixes 8761afd49ebf ("TOMOYO: Allow using owner/group etc. of file objects as conditions.")
Cc Kentaro Takeda <[email protected]>
Cc Tetsuo Handa <[email protected]>

---
Hey everyone,

Spotted this while working on some other fixes. Just an fyi, I'm not subscribed on the mailing list.

Thanks!
Christian

---
 security/tomoyo/condition.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
security/tomoyo/condition.c +11 -1
index f8bcc083bb0d..7e14f8fadbeb 100644
@@ -714,25 +714,35 @@ void tomoyo_get_attributes(struct tomoyo_obj_info *obj)
 {
 	u8 i;
 	struct dentry *dentry = NULL;
+	struct vfsmount *mnt = NULL;
 
 	for (i = 0; i < TOMOYO_MAX_PATH_STAT; i++) {
 		struct inode *inode;
+		struct dentry *parent;
 
 		switch (i) {
 		case TOMOYO_PATH1:
 			dentry = obj->path1.dentry;
 			if (!dentry)
 				continue;
+			mnt = obj->path1.mnt;
 			break;
 		case TOMOYO_PATH2:
 			dentry = obj->path2.dentry;
 			if (!dentry)
 				continue;
+			mnt = obj->path2.mnt;
 			break;
 		default:
 			if (!dentry)
 				continue;
-			dentry = dget_parent(dentry);
+			parent = dget_parent(dentry);
+
+			/* Ensure that parent dentry is reachable. */
+			if (mnt->mnt_root != dentry->d_sb->s_root &&
+			    !is_subdir(dentry, mnt->mnt_root))
+				continue;
+			dentry = parent;
 			break;
 		}
 		inode = d_backing_inode(dentry);

base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56

Signature · 1 line
2.34.1
maintainer
#1 lore raw
Quoted 4 lines · Christian Brauner

When path conditions are specified tomoyo tries to retrieve information about the parent dentry. It currently assumes that the parent dentry is always reachable from the child dentry's mount. This assumption is wrong when bind-mounts are in play:

Thank you for a patch, but I consider that current behavior is correct.

Quoted 1 line

file read /foo/file1 path1.parent.uid=12

The path{1,2}.* and path{1,2}.parent.* conditions use inode's attributes. That is, these conditions are independent with pathname reachability.

---------- Initialization ----------

# mkdir -p /foo /bar
# touch /foo/file1 /bar/file2
# chown 100 /foo
# chown 200 /foo/file1
# chown 300 /bar
# chown 400 /bar/file2

---------- Before doing bind mount ----------

# cat /foo/file1 /bar/file2

---------- Access log of before doing bind mount ----------
#2022/06/07 13:47:14# profile=2 mode=permissive granted=no (global-pid=2757) task={ pid=2757 ppid=2690 uid=0 gid=0 euid=0 egid=0 suid=0 sgid=0 fsuid=0 fsgid=0 } path1={ uid=200 gid=0 ino=2501389 major=8 minor=1 perm=0644 type=file } path1.parent={ uid=100 gid=0 ino=2501384 perm=0755 } <kernel> /usr/sbin/sshd /usr/bin/bash /usr/bin/cat
file read /foo/file1
#2022/06/07 13:47:14# profile=2 mode=permissive granted=no (global-pid=2757) task={ pid=2757 ppid=2690 uid=0 gid=0 euid=0 egid=0 suid=0 sgid=0 fsuid=0 fsgid=0 } path1={ uid=400 gid=0 ino=273557228 major=8 minor=1 perm=0644 type=file } path1.parent={ uid=300 gid=0 ino=273557227 perm=0755 } <kernel> /usr/sbin/sshd /usr/bin/bash /usr/bin/cat
file read /bar/file2

---------- After doing bind mount ----------

# mount --bind /bar/file2 /foo/file1
# cat /foo/file1 /bar/file2

---------- Access log of after doing bind mount ----------
#2022/06/07 13:48:46# profile=2 mode=permissive granted=no (global-pid=2773) task={ pid=2773 ppid=2690 uid=0 gid=0 euid=0 egid=0 suid=0 sgid=0 fsuid=0 fsgid=0 } path1={ uid=400 gid=0 ino=273557228 major=8 minor=1 perm=0644 type=file } path1.parent={ uid=300 gid=0 ino=273557227 perm=0755 } <kernel> /usr/sbin/sshd /usr/bin/bash /usr/bin/cat
file read /foo/file1
#2022/06/07 13:48:46# profile=2 mode=permissive granted=no (global-pid=2773) task={ pid=2773 ppid=2690 uid=0 gid=0 euid=0 egid=0 suid=0 sgid=0 fsuid=0 fsgid=0 } path1={ uid=400 gid=0 ino=273557228 major=8 minor=1 perm=0644 type=file } path1.parent={ uid=300 gid=0 ino=273557227 perm=0755 } <kernel> /usr/sbin/sshd /usr/bin/bash /usr/bin/cat
file read /bar/file2