linux-kernel - Re: [PATCH] selinux: explicitly clean generated av

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHC9VhQ16jkY8qx9=2OJ-A-=9W=Sk_MThg2aqMWqDYrOj7Zzfw@mail.gmail.com>
Date: Tue, 26 Nov 2024 22:03:23 -0500
From: Paul Moore <paul@...l-moore.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Thomas Weißschuh <thomas.weissschuh@...utronix.de>, 
	Stephen Smalley <stephen.smalley.work@...il.com>, Ondrej Mosnacek <omosnace@...hat.com>, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, linux-kbuild@...r.kernel.org, 
	selinux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selinux: explicitly clean generated av_permissions.h

On Wed, Nov 20, 2024 at 6:55 AM Masahiro Yamada <masahiroy@...nel.org> wrote:
> On Wed, Nov 20, 2024 at 6:15 PM Thomas Weißschuh
> <thomas.weissschuh@...utronix.de> wrote:
> >
> > av_permissions.h is not declared as a target and therefore won't be
> > added to clean-files automatically by kbuild.
> > For details why it is not a target see the Makefile itself.
> >
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
> > ---
> >  security/selinux/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/security/selinux/Makefile b/security/selinux/Makefile
> > index 86f0575f670da66a9dc57e13a236d6a5551af38e..58129a7c8cfa08f9caf5444f7df776f41056b77a 100644
> > --- a/security/selinux/Makefile
> > +++ b/security/selinux/Makefile
> > @@ -41,5 +41,8 @@ targets += flask.h
> >  $(obj)/flask.h: $(obj)/genheaders FORCE
> >         $(call if_changed,genhdrs)
> >
> > +# see the note above, remove this line
> > +clean-files += av_permissions.h
> > +
> >  hostprogs := genheaders
> >  HOST_EXTRACFLAGS += -I$(srctree)/security/selinux/include
>
> Presumably, the attached fixup.diff (comment in 'targets' assignment)
> would align with the intention of the maintainer of this Makefile
> because you can do
>
>   targets += $(genhdrs)
>
> without the need of the grouped target feature.
> 'make clean' removes files listed in 'targets'.

[SIDE NOTE: thank you for the comments/patch, but please don't post
patches as attachments, it adds extra steps to review/reply which I
find annoying]

This approach looks a bit cleaner to me.  I thought there was a reason
why we only included flask.h in $targets, but I can't remember it well
enough to know what it was or even if my memory is correct on this :)

> BTW, the NOTE in this Makefile is not true.
>   https://github.com/torvalds/linux/blob/v6.12/security/selinux/Makefile#L7
>
> Even if you use GNU Make 4.3, the grouped target does not work with
> the if_changed macro.
>
> With GNU Make 4.4, it will work as a side-effect of commit
> fabb03eac412b5ea19f1a97be31dc8c6fa7fc047

Patches correcting the Makefile comments are welcome, especially if
what the comments are suggesting for future revisions is incorrect :)

> IMHO, I do not see much benefits for using the group target in this case
> because you can still generate flask.h and av_permissions.h
> separately.

True, although a change in the classmap.h file is going to result in
both being generated; generating only one file is going to be very
rare and not something a normal developer would see.

-- 
paul-moore.com