Re: [PATCH v2 1/2] checkpolicy: extend bad-data tests for type, role, and user errors

Akhil Kohli <[email protected]> Tue, 28 Jul 2026 18:55:05 +0200
Newsgroups org.kernel.vger.selinux
Message-ID <CANu4e6OvTDOZGiAxkE99KZegX95Ro85nugw97+NQ8rremrBxhQ@mail.gmail.com>
Agreed. The BASEDIR soft-resolve is now in patch 1/2 so that patch
passes CI by itself; patch 2 remains the robustness / large-input
coverage.
Posted as [PATCH v3 0/2] (and 1/2, 2/2) to the list.

On Tue, Jul 28, 2026 at 4:28 PM Stephen Smalley
<[email protected]> wrote:
>
> On Tue, Jul 28, 2026 at 8:34 AM Stephen Smalley
> <[email protected]> wrote:
> >
> > On Mon, Jul 27, 2026 at 6:33 PM Akhil Kohli <[email protected]> wrote:
> > >
> > > Add negative .te fixtures for duplicate type/attribute declarations,
> > > type/attribute name conflicts, invalid type names, and malformed role
> > > and user stanzas. Run checkmodule negative tests as non-root in CI so
> > > the unreadable .te case is exercised outside root-only skips.
> > >
> > > Signed-off-by: Akhil Kohli <[email protected]>
> > > ---
> > >
> > > diff --git a/checkpolicy/tests/negative/bad_role.te b/checkpolicy/tests/negative/bad_role.te
> > > new file mode 100644
> > > index 00000000..3aae2719
> > > --- /dev/null
> > > +++ b/checkpolicy/tests/negative/bad_role.te
> > > @@ -0,0 +1,12 @@
> > > +module bad_role 1.0;
> > > +
> > > +require {
> > > +       type foo_t;
> > > +       class file { read };
> > > +}
> > > +
> > > +role bad_role_r types ;
> > > +
> > > +type foo_t;
> > > +
> > > +allow foo_t foo_t:file read;
> > > diff --git a/checkpolicy/tests/negative/bad_user.te b/checkpolicy/tests/negative/bad_user.te
> > > new file mode 100644
> > > index 00000000..e7460dd7
> > > --- /dev/null
> > > +++ b/checkpolicy/tests/negative/bad_user.te
> > > @@ -0,0 +1,3 @@
> > > +module bad_user 1.0;
> > > +
> > > +user bad_user_u garbage_token;
> >
> > You might have missed my earlier comment, but again:
> > this fails NOT on "garbage_token" but on "user" because of
> > the lack of any "requires" block to meet the grammar
> > requirement for avrule_decls. Is that intentional?
>
> Also, you didn't fix the CI breakage in this first patch, only the
> second - it would be better if you could ensure each patch passes CI
> on its own.
>