linux-kernel - Re: [PATCH v3] add group restriction bitmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241001130236.GB23907@redhat.com>
Date: Tue, 1 Oct 2024 15:02:36 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: stsp <stsp2@...dex.ru>
Cc: linux-kernel@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
	Jens Axboe <axboe@...nel.dk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Florent Revest <revest@...omium.org>, Kees Cook <kees@...nel.org>,
	Palmer Dabbelt <palmer@...osinc.com>,
	Charlie Jenkins <charlie@...osinc.com>,
	Benjamin Gray <bgray@...ux.ibm.com>, Helge Deller <deller@....de>,
	Zev Weiss <zev@...ilderbeest.net>,
	Samuel Holland <samuel.holland@...ive.com>,
	linux-fsdevel@...r.kernel.org,
	Eric Biederman <ebiederm@...ssion.com>,
	Andy Lutomirski <luto@...nel.org>,
	Josh Triplett <josh@...htriplett.org>
Subject: Re: [PATCH v3] add group restriction bitmap

On 10/01, stsp wrote:
>
> 01.10.2024 14:15, Oleg Nesterov пишет:
> >Suppose we change groups_search()
> >
> >	--- a/kernel/groups.c
> >	+++ b/kernel/groups.c
> >	@@ -104,8 +104,11 @@ int groups_search(const struct group_info *group_info, kgid_t grp)
> >				left = mid + 1;
> >			else if (gid_lt(grp, group_info->gid[mid]))
> >				right = mid;
> >	-		else
> >	-			return 1;
> >	+		else {
> >	+			bool r = mid < BITS_PER_LONG &&
> >	+				 test_bit(mid, &group_info->restrict_bitmap);
> >	+			return r ? -1 : 1;
> >	+		}
> >		}
> >		return 0;
> >	 }
> >
> >so that it returns, say, -1 if the found grp is restricted.
> >
> >Then everything else can be greatly simplified, afaics...
> This will mean updating all callers
> of groups_search(), in_group_p(),
> in_egroup_p(), vfsxx_in_group_p()

Why? I think with this change you do not need to touch in_group_p/etc at all.

> if in_group_p() returns -1 for not found
> and 0 for gid,

With the the change above in_group_p() returns 0 if not found, !0 otherwise.
It returns -1 if grp != cred->fsgid and the found grp is restricted.

So acl_permission_check() can simply do

	if (mask & (mode ^ (mode >> 3))) {
		vfsgid_t vfsgid = i_gid_into_vfsgid(idmap, inode);
		int xxx = vfsgid_in_group_p(vfsgid);

		if (xxx) {
			if (mask & ~(mode >> 3))
				return -EACCES;
			if (xxx > 0)
				return 0;
			/* If we hit restrict_bitmap, then check Others. */
		}
	}

> So unless I am missing your intention,
> this isn't really helping.

OK, please forget, perhaps I missed something.

Oleg.