[PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Inseob Kim]

This fixes a bug of match_wildcard that incorrectly handles trailing
asterisks. For example, `match_wildcard("abc**", "abc")` must return
true, but it returns false.

Signed-off-by: Inseob Kim <inseob@google.com>
---
v2:
  - Added Cc. No changes to the code.
---
 lib/parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/parser.c b/lib/parser.c
index 73e8f8e5be73..62da0ac0d438 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
 		}
 	}
 
-	if (*p == '*')
+	while (*p == '*')
 		++p;
 	return !*p;
 }
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Andrew Morton]

On Thu, 26 Mar 2026 11:06:04 +0900 Inseob Kim <inseob@google.com> wrote:

> This fixes a bug of match_wildcard that incorrectly handles trailing
> asterisks. For example, `match_wildcard("abc**", "abc")` must return
> true, but it returns false.
> 
> Signed-off-by: Inseob Kim <inseob@google.com>
> ---
> v2:
>   - Added Cc. No changes to the code.
> ---
>  lib/parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/parser.c b/lib/parser.c
> index 73e8f8e5be73..62da0ac0d438 100644
> --- a/lib/parser.c
> +++ b/lib/parser.c
> @@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
>  		}
>  	}
>  
> -	if (*p == '*')
> +	while (*p == '*')
>  		++p;
>  	return !*p;
>  }

Thanks, looks right.

We don't appear to have any selftesting for this code.

Should all of parser.c actually exist?  Some of it is a subset of
lib/glob.c?

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Inseob Kim]

On Thu, Mar 26, 2026 at 11:25 AM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Thu, 26 Mar 2026 11:06:04 +0900 Inseob Kim <inseob@google.com> wrote:
>
> > This fixes a bug of match_wildcard that incorrectly handles trailing
> > asterisks. For example, `match_wildcard("abc**", "abc")` must return
> > true, but it returns false.
> >
> > Signed-off-by: Inseob Kim <inseob@google.com>
> > ---
> > v2:
> >   - Added Cc. No changes to the code.
> > ---
> >  lib/parser.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/parser.c b/lib/parser.c
> > index 73e8f8e5be73..62da0ac0d438 100644
> > --- a/lib/parser.c
> > +++ b/lib/parser.c
> > @@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
> >               }
> >       }
> >
> > -     if (*p == '*')
> > +     while (*p == '*')
> >               ++p;
> >       return !*p;
> >  }
>
> Thanks, looks right.
>
> We don't appear to have any selftesting for this code.

Would you prefer test cases for match_wildcard?

>
> Should all of parser.c actually exist?  Some of it is a subset of
> lib/glob.c?

Wildcard is definitely a subset of glob, but we're intentionally using
wildcard for genfscon for example
(https://lore.kernel.org/selinux/20250318083139.1515253-1-takayas@chromium.org/).
I'd like to leave the parser.c as is.

-- 
Inseob Kim | Software Engineer | inseob@google.com

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Andrew Morton]

On Thu, 26 Mar 2026 13:12:26 +0900 Inseob Kim <inseob@google.com> wrote:

> On Thu, Mar 26, 2026 at 11:25 AM Andrew Morton
> <akpm@linux-foundation.org> wrote:
> >
> > On Thu, 26 Mar 2026 11:06:04 +0900 Inseob Kim <inseob@google.com> wrote:
> >
> > > This fixes a bug of match_wildcard that incorrectly handles trailing
> > > asterisks. For example, `match_wildcard("abc**", "abc")` must return
> > > true, but it returns false.
> > >
> > > Signed-off-by: Inseob Kim <inseob@google.com>
> > > ---
> > > v2:
> > >   - Added Cc. No changes to the code.
> > > ---
> > >  lib/parser.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/parser.c b/lib/parser.c
> > > index 73e8f8e5be73..62da0ac0d438 100644
> > > --- a/lib/parser.c
> > > +++ b/lib/parser.c
> > > @@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
> > >               }
> > >       }
> > >
> > > -     if (*p == '*')
> > > +     while (*p == '*')
> > >               ++p;
> > >       return !*p;
> > >  }
> >
> > Thanks, looks right.
> >
> > We don't appear to have any selftesting for this code.
> 
> Would you prefer test cases for match_wildcard?

That would of course be wonderful, but such a contribution is unrelated
to this bugfix.  Up to you.

> >
> > Should all of parser.c actually exist?  Some of it is a subset of
> > lib/glob.c?
> 
> Wildcard is definitely a subset of glob, but we're intentionally using
> wildcard for genfscon for example
> (https://lore.kernel.org/selinux/20250318083139.1515253-1-takayas@chromium.org/).
> I'd like to leave the parser.c as is.

That's a nice boot-time improvement, but I don't understand from that
why wildcard is preferable to glob?

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Inseob Kim]

On Thu, Mar 26, 2026 at 1:45 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 26 Mar 2026 13:12:26 +0900 Inseob Kim <inseob@google.com> wrote:
>
> > On Thu, Mar 26, 2026 at 11:25 AM Andrew Morton
> > <akpm@linux-foundation.org> wrote:
> > >
> > > On Thu, 26 Mar 2026 11:06:04 +0900 Inseob Kim <inseob@google.com> wrote:
> > >
> > > > This fixes a bug of match_wildcard that incorrectly handles trailing
> > > > asterisks. For example, `match_wildcard("abc**", "abc")` must return
> > > > true, but it returns false.
> > > >
> > > > Signed-off-by: Inseob Kim <inseob@google.com>
> > > > ---
> > > > v2:
> > > >   - Added Cc. No changes to the code.
> > > > ---
> > > >  lib/parser.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/parser.c b/lib/parser.c
> > > > index 73e8f8e5be73..62da0ac0d438 100644
> > > > --- a/lib/parser.c
> > > > +++ b/lib/parser.c
> > > > @@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
> > > >               }
> > > >       }
> > > >
> > > > -     if (*p == '*')
> > > > +     while (*p == '*')
> > > >               ++p;
> > > >       return !*p;
> > > >  }
> > >
> > > Thanks, looks right.
> > >
> > > We don't appear to have any selftesting for this code.
> >
> > Would you prefer test cases for match_wildcard?
>
> That would of course be wonderful, but such a contribution is unrelated
> to this bugfix.  Up to you.
>
> > >
> > > Should all of parser.c actually exist?  Some of it is a subset of
> > > lib/glob.c?
> >
> > Wildcard is definitely a subset of glob, but we're intentionally using
> > wildcard for genfscon for example
> > (https://lore.kernel.org/selinux/20250318083139.1515253-1-takayas@chromium.org/).
> > I'd like to leave the parser.c as is.
>
> That's a nice boot-time improvement, but I don't understand from that
> why wildcard is preferable to glob?


Sorry for the confusion; let me correct myself. It's not that wildcard
is preferable to glob, but genfs_seclabel_wildcard is already merged
and in use so moving from wildcard to glob would introduce breaking
changes. Let me invite Takaya who possibly has more rationale for the
wildcard.

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Takaya Saeki]

On Thu, Mar 26, 2026 at 1:51 PM Inseob Kim <inseob@google.com> wrote:
>
> On Thu, Mar 26, 2026 at 1:45 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Thu, 26 Mar 2026 13:12:26 +0900 Inseob Kim <inseob@google.com> wrote:
> >
> > > On Thu, Mar 26, 2026 at 11:25 AM Andrew Morton
> > > <akpm@linux-foundation.org> wrote:
> > > >
> > > > On Thu, 26 Mar 2026 11:06:04 +0900 Inseob Kim <inseob@google.com> wrote:
> > > >
> > > > > This fixes a bug of match_wildcard that incorrectly handles trailing
> > > > > asterisks. For example, `match_wildcard("abc**", "abc")` must return
> > > > > true, but it returns false.
> > > > >
> > > > > Signed-off-by: Inseob Kim <inseob@google.com>
> > > > > ---
> > > > > v2:
> > > > >   - Added Cc. No changes to the code.
> > > > > ---
> > > > >  lib/parser.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/lib/parser.c b/lib/parser.c
> > > > > index 73e8f8e5be73..62da0ac0d438 100644
> > > > > --- a/lib/parser.c
> > > > > +++ b/lib/parser.c
> > > > > @@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
> > > > >               }
> > > > >       }
> > > > >
> > > > > -     if (*p == '*')
> > > > > +     while (*p == '*')
> > > > >               ++p;
> > > > >       return !*p;
> > > > >  }
> > > >
> > > > Thanks, looks right.
> > > >
> > > > We don't appear to have any selftesting for this code.
> > >
> > > Would you prefer test cases for match_wildcard?
> >
> > That would of course be wonderful, but such a contribution is unrelated
> > to this bugfix.  Up to you.
> >
> > > >
> > > > Should all of parser.c actually exist?  Some of it is a subset of
> > > > lib/glob.c?
> > >
> > > Wildcard is definitely a subset of glob, but we're intentionally using
> > > wildcard for genfscon for example
> > > (https://lore.kernel.org/selinux/20250318083139.1515253-1-takayas@chromium.org/).
> > > I'd like to leave the parser.c as is.
> >
> > That's a nice boot-time improvement, but I don't understand from that
> > why wildcard is preferable to glob?
>
>
> Sorry for the confusion; let me correct myself. It's not that wildcard
> is preferable to glob, but genfs_seclabel_wildcard is already merged
> and in use so moving from wildcard to glob would introduce breaking
> changes. Let me invite Takaya who possibly has more rationale for the
> wildcard.

Yes, there wasn't strong motivation to avoid glob; wildcards were sufficient,
so we just didn't consider adopting glob.
Since glob is a superset of wildcards and doesn't treat slashes as
special characters,
it would certainly be more convenient to use. We might want to improve SELinux
later including the network interface matching, which also uses
wildcard matching.

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Andrew Morton]

On Fri, 27 Mar 2026 19:11:21 +0900 Takaya Saeki <takayas@google.com> wrote:

> > Sorry for the confusion; let me correct myself. It's not that wildcard
> > is preferable to glob, but genfs_seclabel_wildcard is already merged
> > and in use so moving from wildcard to glob would introduce breaking
> > changes. Let me invite Takaya who possibly has more rationale for the
> > wildcard.
> 
> Yes, there wasn't strong motivation to avoid glob; wildcards were sufficient,
> so we just didn't consider adopting glob.
> Since glob is a superset of wildcards and doesn't treat slashes as
> special characters,
> it would certainly be more convenient to use. We might want to improve SELinux
> later including the network interface matching, which also uses
> wildcard matching.

OK, thanks.

It isn't a big deal at all - I'm just wondering if we have an
opportunity to remove some code by migrating all wildcard users over to
glob.

Re: [PATCH v2] lib: parser: Fix match_wildcard to correctly handle trailing stars

[Josh Law]

On 26 March 2026 02:25:34 GMT, Andrew Morton <akpm@linux-foundation.org> wrote:
>On Thu, 26 Mar 2026 11:06:04 +0900 Inseob Kim <inseob@google.com> wrote:
>
>> This fixes a bug of match_wildcard that incorrectly handles trailing
>> asterisks. For example, `match_wildcard("abc**", "abc")` must return
>> true, but it returns false.
>> 
>> Signed-off-by: Inseob Kim <inseob@google.com>
>> ---
>> v2:
>>   - Added Cc. No changes to the code.
>> ---
>>  lib/parser.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/lib/parser.c b/lib/parser.c
>> index 73e8f8e5be73..62da0ac0d438 100644
>> --- a/lib/parser.c
>> +++ b/lib/parser.c
>> @@ -315,7 +315,7 @@ bool match_wildcard(const char *pattern, const char *str)
>>  		}
>>  	}
>>  
>> -	if (*p == '*')
>> +	while (*p == '*')
>>  		++p;
>>  	return !*p;
>>  }
>
>Thanks, looks right.
>
>We don't appear to have any selftesting for this code.
>
>Should all of parser.c actually exist?  Some of it is a subset of
>lib/glob.c?



Hi!


Reviewed-By: Josh Law <objecting@objecting.org>

Good patch