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.
