On Tue, Jun 21, 2022 at 01:22:30PM -0500, Seth Forshee wrote:
> On Tue, Jun 21, 2022 at 04:14:47PM +0200, Christian Brauner wrote:
> > +/**
> > + * kuid_eq_vfsuid - check whether kuid and vfsuid have the same value
> > + * @kuid: the kuid to compare
> > + * @vfsuid: the vfsuid to compare
> > + *
> > + * Check whether @kuid and @vfsuid have the same values.
> > + *
> > + * Return: true if @kuid and @vfsuid have the same value, false if not.
> > + */
> > +static inline bool kuid_eq_vfsuid(kuid_t kuid, vfsuid_t vfsuid)
> > +{
> > + return __vfsuid_val(vfsuid) == __kuid_val(kuid);
> > +}
>
> Something that I think would be helpful is if this and other comparison
> functions always returned false for comparisons with invalid, e.g.:
>
> static inline bool kuid_eq_vfsuid(kuid_t kuid, vfsuid_t vfsuid)
> {
> return vfsuid_valid(vfsuid) && __vfsuid_val(vfsuid) == __kuid_val(kuid);
> }
>
> I can't imagine any cases where we would want even two invalid ids to
> evaluate as being equal, and so as it is now we have to take great care
> to ensure that we never end up with comparisons between two invalid ids.
>
> Honestly I'd like to see that for kuid_t comparisons too, but I suppose
> that's a little out of scope here.
Yes, I agree.
That goes back to our discussion yesterday. Thank you for that idea!
As I've mentioned, I'd like to do to this carefully by applying a final
patch on top of the whole series that adds an additional vfsuid_valid()
check to all equality helpers. In case we see a regression we can then
easily revert the single commit on top.
Christian
