| Message ID | 20230925172037.work.853-kees@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 921f15fe8c8cf9543665bdc556e080a5ecbc34b3 |
| Headers | show |
| Series | MAINTAINERS: hardening: Add __counted_by regex | expand |
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
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
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
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
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(+)