[PATCH] MAINTAINERS: hardening: Add __counted_by regex

[PATCH] MAINTAINERS: hardening: Add __counted_by regex

[Kees Cook]

Since __counted_by annotations may also require that code be changed to
get initialization ordering correct, let's get an extra group of eyes on
code that is working on these annotations.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 737dcc7a2155..741285b8246e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11405,6 +11405,7 @@ F:	kernel/configs/hardening.config
 F:	mm/usercopy.c
 K:	\b(add|choose)_random_kstack_offset\b
 K:	\b__check_(object_size|heap_object)\b
+K:	\b__counted_by\b
 
 KERNEL JANITORS
 L:	kernel-janitors@vger.kernel.org
-- 
2.34.1

Re: [PATCH] MAINTAINERS: hardening: Add __counted_by regex

[Eric Biggers]

Hi Kees,

On Mon, Sep 25, 2023 at 10:20:41AM -0700, Kees Cook wrote:
> Since __counted_by annotations may also require that code be changed to
> get initialization ordering correct, let's get an extra group of eyes on
> code that is working on these annotations.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 737dcc7a2155..741285b8246e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11405,6 +11405,7 @@ F:	kernel/configs/hardening.config
>  F:	mm/usercopy.c
>  K:	\b(add|choose)_random_kstack_offset\b
>  K:	\b__check_(object_size|heap_object)\b
> +K:	\b__counted_by\b
>  

Are you sure you want to volunteer to maintain every file that contains
"__counted_by"?  That's what "K" does; get_maintainer.pl will list you (and
linux-hardening@vger.kernel.org) for every such file.

Other users of "K" have been surprised by this behavior.  It seems that most
people expect it to only apply to patches, not to files.  Given that you're
interested in using this functionality, have you considered updating
checkpatch.pl to handle it in the way that you probably expect that it works?

- Eric

Re: [PATCH] MAINTAINERS: hardening: Add __counted_by regex

[Justin Stitt]

On Tue, Sep 26, 2023 at 1:57 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> Hi Kees,
>
> On Mon, Sep 25, 2023 at 10:20:41AM -0700, Kees Cook wrote:
> > Since __counted_by annotations may also require that code be changed to
> > get initialization ordering correct, let's get an extra group of eyes on
> > code that is working on these annotations.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  MAINTAINERS | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 737dcc7a2155..741285b8246e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11405,6 +11405,7 @@ F:    kernel/configs/hardening.config
> >  F:   mm/usercopy.c
> >  K:   \b(add|choose)_random_kstack_offset\b
> >  K:   \b__check_(object_size|heap_object)\b
> > +K:   \b__counted_by\b
> >
>
> Are you sure you want to volunteer to maintain every file that contains
> "__counted_by"?  That's what "K" does; get_maintainer.pl will list you (and
> linux-hardening@vger.kernel.org) for every such file.

Do people call get_maintainer.pl on specific tree files as opposed to
invoking it against a .patch file? In the event of the .patch file
"K:" should only pick-up what's in the patch and not read into the
files outside of the context that the diff provides.

If needed, I could send a patch adding a "D:" which would only
consider patches and not tree files -- reducing noise.

>
> Other users of "K" have been surprised by this behavior.  It seems that most
> people expect it to only apply to patches, not to files.  Given that you're
> interested in using this functionality, have you considered updating
> checkpatch.pl to handle it in the way that you probably expect that it works?
>
> - Eric
>

Thanks
Justin

Re: [PATCH] MAINTAINERS: hardening: Add __counted_by regex

[Justin Stitt]

On Tue, Sep 26, 2023 at 5:35 PM Justin Stitt <justinstitt@google.com> wrote:
>
> On Tue, Sep 26, 2023 at 1:57 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Hi Kees,
> >
> > On Mon, Sep 25, 2023 at 10:20:41AM -0700, Kees Cook wrote:
> > > Since __counted_by annotations may also require that code be changed to
> > > get initialization ordering correct, let's get an extra group of eyes on
> > > code that is working on these annotations.
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > >  MAINTAINERS | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 737dcc7a2155..741285b8246e 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -11405,6 +11405,7 @@ F:    kernel/configs/hardening.config
> > >  F:   mm/usercopy.c
> > >  K:   \b(add|choose)_random_kstack_offset\b
> > >  K:   \b__check_(object_size|heap_object)\b
> > > +K:   \b__counted_by\b
> > >
> >
> > Are you sure you want to volunteer to maintain every file that contains
> > "__counted_by"?  That's what "K" does; get_maintainer.pl will list you (and
> > linux-hardening@vger.kernel.org) for every such file.
>
> Do people call get_maintainer.pl on specific tree files as opposed to
> invoking it against a .patch file? In the event of the .patch file
> "K:" should only pick-up what's in the patch and not read into the
> files outside of the context that the diff provides.

FWIW, b4 just uses the patches and not entire files:

   ...
    try:
        tos, ccs, tag_msg, patches = get_prep_branch_as_patches()
    except RuntimeError:
        logger.info('No commits in branch')
        return

    logger.info('Collecting To/Cc addresses')
    # Go through the messages to make to/cc headers
    for commit, msg in patches:
        if not msg or not commit:
            continue

        logger.debug('Collecting from: %s', msg.get('subject'))
        msgbytes = msg.as_bytes()
        ...


>
> If needed, I could send a patch adding a "D:" which would only
> consider patches and not tree files -- reducing noise.
>
> >
> > Other users of "K" have been surprised by this behavior.  It seems that most
> > people expect it to only apply to patches, not to files.  Given that you're
> > interested in using this functionality, have you considered updating
> > checkpatch.pl to handle it in the way that you probably expect that it works?
> >
> > - Eric
> >
>
> Thanks
> Justin