* [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
@ 2018-09-09 14:04 Loic
2018-09-17 11:49 ` Greg KH
2018-09-17 13:58 ` Greg KH
0 siblings, 2 replies; 16+ messages in thread
From: Loic @ 2018-09-09 14:04 UTC (permalink / raw)
To: stable, arnd, john.johansen, james.l.morris
Hello,
Tested without any problem so please picked up this for 4.4 to fix the
problem.
The patch below is slightly modified to adapt to this version.
[ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
The newly added Kconfig option could never work and just causes a build
error
when disabled:
security/apparmor/lsm.c:675:25: error:
'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a
function)
bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
The problem is that the macro undefined in this case, and we need to use
the IS_ENABLED()
helper to turn it into a boolean constant.
Another minor problem with the original patch is that the option is even
offered
in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides
the option
in that case.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
hashing is used")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
---
diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
--- a/security/apparmor/crypto.c
+++ b/security/apparmor/crypto.c
@@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
int error = -ENOMEM;
u32 le32_version = cpu_to_le32(version);
+ if (!aa_g_hash_policy)
+ return 0;
+
if (!apparmor_tfm)
return 0;
diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
module_param_call(mode, param_set_mode, param_get_mode,
&aa_g_profile_mode, S_IRUSR | S_IWUSR);
+#ifdef CONFIG_SECURITY_APPARMOR_HASH
+/* whether policy verification hashing is enabled */
+bool aa_g_hash_policy =
IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
+module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
S_IWUSR);
+#endif
+
/* Debug mode */
bool aa_g_debug;
module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
---
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-09 14:04 [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling Loic
@ 2018-09-17 11:49 ` Greg KH
2018-09-17 13:40 ` John Johansen
2018-09-17 13:58 ` Greg KH
1 sibling, 1 reply; 16+ messages in thread
From: Greg KH @ 2018-09-17 11:49 UTC (permalink / raw)
To: Loic; +Cc: stable, arnd, john.johansen, james.l.morris
On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> Hello,
>
> Tested without any problem so please picked up this for 4.4 to fix the
> problem.
> The patch below is slightly modified to adapt to this version.
I would like to get an ack from one of the developers/maintainers of
this patch before I accept it, as it does differ from the in-tree
version a bit.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 11:49 ` Greg KH
@ 2018-09-17 13:40 ` John Johansen
2018-09-17 13:58 ` Greg KH
0 siblings, 1 reply; 16+ messages in thread
From: John Johansen @ 2018-09-17 13:40 UTC (permalink / raw)
To: Greg KH, Loic; +Cc: stable, arnd, james.l.morris
On 09/17/2018 04:49 AM, Greg KH wrote:
> On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
>> Hello,
>>
>> Tested without any problem so please picked up this for 4.4 to fix the
>> problem.
>> The patch below is slightly modified to adapt to this version.
>
> I would like to get an ack from one of the developers/maintainers of
> this patch before I accept it, as it does differ from the in-tree
> version a bit.
>
Greg the patch is good. Full explanation below
cherry-picking the original patch to 4.4, I get 2 conflicts
both modified: security/apparmor/lsm.c
both modified: security/apparmor/policy_unpack.c
the lsm.c conflict is do to surrounding code changes and is handled
correctly. The patch drops the policy_unpack.c change
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index b9b1c66a32a5..138120698f83 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -778,8 +778,7 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns)
if (error)
goto fail_profile;
- if (aa_g_hash_policy)
- error = aa_calc_profile_hash(profile, e.version, start,
+ error = aa_calc_profile_hash(profile, e.version, start,
e.pos - start);
if (error)
goto fail_profile;
The conflict is caused by commit
31f75bfecd9cef7d485b1cda3c6c38cc0b4a5c6c in 4.11, which adds the
conditional check that the upstream version of this patch is dropping
again.
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 13:40 ` John Johansen
@ 2018-09-17 13:58 ` Greg KH
0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2018-09-17 13:58 UTC (permalink / raw)
To: John Johansen; +Cc: Loic, stable, arnd, james.l.morris
On Mon, Sep 17, 2018 at 06:40:20AM -0700, John Johansen wrote:
> On 09/17/2018 04:49 AM, Greg KH wrote:
> > On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> >> Hello,
> >>
> >> Tested without any problem so please picked up this for 4.4 to fix the
> >> problem.
> >> The patch below is slightly modified to adapt to this version.
> >
> > I would like to get an ack from one of the developers/maintainers of
> > this patch before I accept it, as it does differ from the in-tree
> > version a bit.
> >
>
> Greg the patch is good. Full explanation below
>
>
> cherry-picking the original patch to 4.4, I get 2 conflicts
>
> both modified: security/apparmor/lsm.c
> both modified: security/apparmor/policy_unpack.c
>
> the lsm.c conflict is do to surrounding code changes and is handled
> correctly. The patch drops the policy_unpack.c change
Great, thanks for reviewing this!
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-09 14:04 [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling Loic
2018-09-17 11:49 ` Greg KH
@ 2018-09-17 13:58 ` Greg KH
2018-09-17 19:45 ` Loic
1 sibling, 1 reply; 16+ messages in thread
From: Greg KH @ 2018-09-17 13:58 UTC (permalink / raw)
To: Loic; +Cc: stable, arnd, john.johansen, james.l.morris
On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> Hello,
>
> Tested without any problem so please picked up this for 4.4 to fix the
> problem.
> The patch below is slightly modified to adapt to this version.
>
> [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
>
> The newly added Kconfig option could never work and just causes a build
> error
> when disabled:
>
> security/apparmor/lsm.c:675:25: error:
> 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
>
> The problem is that the macro undefined in this case, and we need to use the
> IS_ENABLED()
> helper to turn it into a boolean constant.
>
> Another minor problem with the original patch is that the option is even
> offered
> in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the
> option
> in that case.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
> hashing is used")
> Signed-off-by: John Johansen <john.johansen@canonical.com>
> Signed-off-by: James Morris <james.l.morris@oracle.com>
> ---
> diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> --- a/security/apparmor/crypto.c
> +++ b/security/apparmor/crypto.c
> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> int error = -ENOMEM;
> u32 le32_version = cpu_to_le32(version);
>
> + if (!aa_g_hash_policy)
> + return 0;
> +
> if (!apparmor_tfm)
> return 0;
>
> diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
> module_param_call(mode, param_set_mode, param_get_mode,
> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>
> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> +/* whether policy verification hashing is enabled */
> +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
> S_IWUSR);
> +#endif
> +
> /* Debug mode */
> bool aa_g_debug;
> module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> ---
The patch is whitespace corrupted and can not be applied :(
Can you fix that up and resend it so that I can apply it?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 13:58 ` Greg KH
@ 2018-09-17 19:45 ` Loic
2018-09-17 21:15 ` Greg KH
0 siblings, 1 reply; 16+ messages in thread
From: Loic @ 2018-09-17 19:45 UTC (permalink / raw)
To: Greg KH; +Cc: stable, arnd, john.johansen, james.l.morris
On Mon, 17 Sep 2018 15:58:56 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> > Hello,
> >
> > Tested without any problem so please picked up this for 4.4 to fix the
> > problem.
> > The patch below is slightly modified to adapt to this version.
> >
> > [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
> >
> > The newly added Kconfig option could never work and just causes a build
> > error
> > when disabled:
> >
> > security/apparmor/lsm.c:675:25: error:
> > 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> > bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
> >
> > The problem is that the macro undefined in this case, and we need to use the
> > IS_ENABLED()
> > helper to turn it into a boolean constant.
> >
> > Another minor problem with the original patch is that the option is even
> > offered
> > in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the
> > option
> > in that case.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
> > hashing is used")
> > Signed-off-by: John Johansen <john.johansen@canonical.com>
> > Signed-off-by: James Morris <james.l.morris@oracle.com>
> > ---
> > diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> > --- a/security/apparmor/crypto.c
> > +++ b/security/apparmor/crypto.c
> > @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> > int error = -ENOMEM;
> > u32 le32_version = cpu_to_le32(version);
> >
> > + if (!aa_g_hash_policy)
> > + return 0;
> > +
> > if (!apparmor_tfm)
> > return 0;
> >
> > diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > --- a/security/apparmor/lsm.c
> > +++ b/security/apparmor/lsm.c
> > @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
> > module_param_call(mode, param_set_mode, param_get_mode,
> > &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> >
> > +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> > +/* whether policy verification hashing is enabled */
> > +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> > +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
> > S_IWUSR);
> > +#endif
> > +
> > /* Debug mode */
> > bool aa_g_debug;
> > module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> > ---
>
> The patch is whitespace corrupted and can not be applied :(
Sorry, I noticed the problem afterwards. I opened a bug report to try to fix my mail client:
https://github.com/roundcube/roundcubemail/issues/6438
>
> Can you fix that up and resend it so that I can apply it?
No problem. Thanks for all.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
---
diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
--- a/security/apparmor/crypto.c
+++ b/security/apparmor/crypto.c
@@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
int error = -ENOMEM;
u32 le32_version = cpu_to_le32(version);
+ if (!aa_g_hash_policy)
+ return 0;
+
if (!apparmor_tfm)
return 0;
diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -692,6 +694,12 @@ enum profile_mode aa_g_profile_mode = AP
module_param_call(mode, param_set_mode, param_get_mode,
&aa_g_profile_mode, S_IRUSR | S_IWUSR);
+#ifdef CONFIG_SECURITY_APPARMOR_HASH
+/* whether policy verification hashing is enabled */
+bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
+module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
+#endif
+
/* Debug mode */
bool aa_g_debug;
module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 19:45 ` Loic
@ 2018-09-17 21:15 ` Greg KH
2018-09-17 21:37 ` Greg KH
0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2018-09-17 21:15 UTC (permalink / raw)
To: Loic; +Cc: stable, arnd, john.johansen, james.l.morris
On Mon, Sep 17, 2018 at 09:45:47PM +0200, Loic wrote:
> On Mon, 17 Sep 2018 15:58:56 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> > > Hello,
> > >
> > > Tested without any problem so please picked up this for 4.4 to fix the
> > > problem.
> > > The patch below is slightly modified to adapt to this version.
> > >
> > > [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
> > >
> > > The newly added Kconfig option could never work and just causes a build
> > > error
> > > when disabled:
> > >
> > > security/apparmor/lsm.c:675:25: error:
> > > 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> > > bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
> > >
> > > The problem is that the macro undefined in this case, and we need to use the
> > > IS_ENABLED()
> > > helper to turn it into a boolean constant.
> > >
> > > Another minor problem with the original patch is that the option is even
> > > offered
> > > in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the
> > > option
> > > in that case.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
> > > hashing is used")
> > > Signed-off-by: John Johansen <john.johansen@canonical.com>
> > > Signed-off-by: James Morris <james.l.morris@oracle.com>
> > > ---
> > > diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> > > --- a/security/apparmor/crypto.c
> > > +++ b/security/apparmor/crypto.c
> > > @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> > > int error = -ENOMEM;
> > > u32 le32_version = cpu_to_le32(version);
> > >
> > > + if (!aa_g_hash_policy)
> > > + return 0;
> > > +
> > > if (!apparmor_tfm)
> > > return 0;
> > >
> > > diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > > --- a/security/apparmor/lsm.c
> > > +++ b/security/apparmor/lsm.c
> > > @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
> > > module_param_call(mode, param_set_mode, param_get_mode,
> > > &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> > >
> > > +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> > > +/* whether policy verification hashing is enabled */
> > > +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> > > +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
> > > S_IWUSR);
> > > +#endif
> > > +
> > > /* Debug mode */
> > > bool aa_g_debug;
> > > module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> > > ---
> >
> > The patch is whitespace corrupted and can not be applied :(
>
> Sorry, I noticed the problem afterwards. I opened a bug report to try to fix my mail client:
> https://github.com/roundcube/roundcubemail/issues/6438
>
> >
> > Can you fix that up and resend it so that I can apply it?
>
> No problem. Thanks for all.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
> Signed-off-by: John Johansen <john.johansen@canonical.com>
> Signed-off-by: James Morris <james.l.morris@oracle.com>
> ---
> diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> --- a/security/apparmor/crypto.c
> +++ b/security/apparmor/crypto.c
> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> int error = -ENOMEM;
> u32 le32_version = cpu_to_le32(version);
>
> + if (!aa_g_hash_policy)
> + return 0;
> +
> if (!apparmor_tfm)
> return 0;
>
> diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -692,6 +694,12 @@ enum profile_mode aa_g_profile_mode = AP
> module_param_call(mode, param_set_mode, param_get_mode,
> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>
> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> +/* whether policy verification hashing is enabled */
> +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
> +#endif
> +
> /* Debug mode */
> bool aa_g_debug;
> module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
THanks, that worked, now queued up.
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 21:15 ` Greg KH
@ 2018-09-17 21:37 ` Greg KH
2018-09-17 21:56 ` Nathan Chancellor
0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2018-09-17 21:37 UTC (permalink / raw)
To: Loic; +Cc: stable, arnd, john.johansen, james.l.morris
On Mon, Sep 17, 2018 at 11:15:42PM +0200, Greg KH wrote:
> On Mon, Sep 17, 2018 at 09:45:47PM +0200, Loic wrote:
> > On Mon, 17 Sep 2018 15:58:56 +0200
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > > On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> > > > Hello,
> > > >
> > > > Tested without any problem so please picked up this for 4.4 to fix the
> > > > problem.
> > > > The patch below is slightly modified to adapt to this version.
> > > >
> > > > [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
> > > >
> > > > The newly added Kconfig option could never work and just causes a build
> > > > error
> > > > when disabled:
> > > >
> > > > security/apparmor/lsm.c:675:25: error:
> > > > 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> > > > bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
> > > >
> > > > The problem is that the macro undefined in this case, and we need to use the
> > > > IS_ENABLED()
> > > > helper to turn it into a boolean constant.
> > > >
> > > > Another minor problem with the original patch is that the option is even
> > > > offered
> > > > in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the
> > > > option
> > > > in that case.
> > > >
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
> > > > hashing is used")
> > > > Signed-off-by: John Johansen <john.johansen@canonical.com>
> > > > Signed-off-by: James Morris <james.l.morris@oracle.com>
> > > > ---
> > > > diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> > > > --- a/security/apparmor/crypto.c
> > > > +++ b/security/apparmor/crypto.c
> > > > @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> > > > int error = -ENOMEM;
> > > > u32 le32_version = cpu_to_le32(version);
> > > >
> > > > + if (!aa_g_hash_policy)
> > > > + return 0;
> > > > +
> > > > if (!apparmor_tfm)
> > > > return 0;
> > > >
> > > > diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > > > --- a/security/apparmor/lsm.c
> > > > +++ b/security/apparmor/lsm.c
> > > > @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
> > > > module_param_call(mode, param_set_mode, param_get_mode,
> > > > &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> > > >
> > > > +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> > > > +/* whether policy verification hashing is enabled */
> > > > +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> > > > +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
> > > > S_IWUSR);
> > > > +#endif
> > > > +
> > > > /* Debug mode */
> > > > bool aa_g_debug;
> > > > module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> > > > ---
> > >
> > > The patch is whitespace corrupted and can not be applied :(
> >
> > Sorry, I noticed the problem afterwards. I opened a bug report to try to fix my mail client:
> > https://github.com/roundcube/roundcubemail/issues/6438
> >
> > >
> > > Can you fix that up and resend it so that I can apply it?
> >
> > No problem. Thanks for all.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
> > Signed-off-by: John Johansen <john.johansen@canonical.com>
> > Signed-off-by: James Morris <james.l.morris@oracle.com>
> > ---
> > diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> > --- a/security/apparmor/crypto.c
> > +++ b/security/apparmor/crypto.c
> > @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> > int error = -ENOMEM;
> > u32 le32_version = cpu_to_le32(version);
> >
> > + if (!aa_g_hash_policy)
> > + return 0;
> > +
> > if (!apparmor_tfm)
> > return 0;
> >
> > diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > --- a/security/apparmor/lsm.c
> > +++ b/security/apparmor/lsm.c
> > @@ -692,6 +694,12 @@ enum profile_mode aa_g_profile_mode = AP
> > module_param_call(mode, param_set_mode, param_get_mode,
> > &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> >
> > +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> > +/* whether policy verification hashing is enabled */
> > +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> > +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
> > +#endif
> > +
> > /* Debug mode */
> > bool aa_g_debug;
> > module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
>
> THanks, that worked, now queued up.
And dropped as this patch broke the build. I'm guessing this really
isn't needed if you didn't even test it worked :(
Please fix all of this up, test it properly, and then send it as a
"clean" patch that I can apply easily.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 21:37 ` Greg KH
@ 2018-09-17 21:56 ` Nathan Chancellor
2018-09-17 22:12 ` John Johansen
0 siblings, 1 reply; 16+ messages in thread
From: Nathan Chancellor @ 2018-09-17 21:56 UTC (permalink / raw)
To: Greg KH; +Cc: Loic, stable, arnd, john.johansen, james.l.morris
On Mon, Sep 17, 2018 at 11:37:54PM +0200, Greg KH wrote:
> On Mon, Sep 17, 2018 at 11:15:42PM +0200, Greg KH wrote:
> > On Mon, Sep 17, 2018 at 09:45:47PM +0200, Loic wrote:
> > > On Mon, 17 Sep 2018 15:58:56 +0200
> > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > > On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
> > > > > Hello,
> > > > >
> > > > > Tested without any problem so please picked up this for 4.4 to fix the
> > > > > problem.
> > > > > The patch below is slightly modified to adapt to this version.
> > > > >
> > > > > [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
> > > > >
> > > > > The newly added Kconfig option could never work and just causes a build
> > > > > error
> > > > > when disabled:
> > > > >
> > > > > security/apparmor/lsm.c:675:25: error:
> > > > > 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> > > > > bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
> > > > >
> > > > > The problem is that the macro undefined in this case, and we need to use the
> > > > > IS_ENABLED()
> > > > > helper to turn it into a boolean constant.
> > > > >
> > > > > Another minor problem with the original patch is that the option is even
> > > > > offered
> > > > > in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the
> > > > > option
> > > > > in that case.
> > > > >
> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > > Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
> > > > > hashing is used")
> > > > > Signed-off-by: John Johansen <john.johansen@canonical.com>
> > > > > Signed-off-by: James Morris <james.l.morris@oracle.com>
> > > > > ---
> > > > > diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> > > > > --- a/security/apparmor/crypto.c
> > > > > +++ b/security/apparmor/crypto.c
> > > > > @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> > > > > int error = -ENOMEM;
> > > > > u32 le32_version = cpu_to_le32(version);
> > > > >
> > > > > + if (!aa_g_hash_policy)
> > > > > + return 0;
> > > > > +
> > > > > if (!apparmor_tfm)
> > > > > return 0;
> > > > >
> > > > > diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > > > > --- a/security/apparmor/lsm.c
> > > > > +++ b/security/apparmor/lsm.c
> > > > > @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
> > > > > module_param_call(mode, param_set_mode, param_get_mode,
> > > > > &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> > > > >
> > > > > +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> > > > > +/* whether policy verification hashing is enabled */
> > > > > +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> > > > > +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
> > > > > S_IWUSR);
> > > > > +#endif
> > > > > +
> > > > > /* Debug mode */
> > > > > bool aa_g_debug;
> > > > > module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> > > > > ---
> > > >
> > > > The patch is whitespace corrupted and can not be applied :(
> > >
> > > Sorry, I noticed the problem afterwards. I opened a bug report to try to fix my mail client:
> > > https://github.com/roundcube/roundcubemail/issues/6438
> > >
> > > >
> > > > Can you fix that up and resend it so that I can apply it?
> > >
> > > No problem. Thanks for all.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
> > > Signed-off-by: John Johansen <john.johansen@canonical.com>
> > > Signed-off-by: James Morris <james.l.morris@oracle.com>
> > > ---
> > > diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> > > --- a/security/apparmor/crypto.c
> > > +++ b/security/apparmor/crypto.c
> > > @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
> > > int error = -ENOMEM;
> > > u32 le32_version = cpu_to_le32(version);
> > >
> > > + if (!aa_g_hash_policy)
> > > + return 0;
> > > +
> > > if (!apparmor_tfm)
> > > return 0;
> > >
> > > diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > > --- a/security/apparmor/lsm.c
> > > +++ b/security/apparmor/lsm.c
> > > @@ -692,6 +694,12 @@ enum profile_mode aa_g_profile_mode = AP
> > > module_param_call(mode, param_set_mode, param_get_mode,
> > > &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> > >
> > > +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> > > +/* whether policy verification hashing is enabled */
> > > +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> > > +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
> > > +#endif
> > > +
> > > /* Debug mode */
> > > bool aa_g_debug;
> > > module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> >
> > THanks, that worked, now queued up.
>
> And dropped as this patch broke the build. I'm guessing this really
> isn't needed if you didn't even test it worked :(
>
> Please fix all of this up, test it properly, and then send it as a
> "clean" patch that I can apply easily.
>
> thanks,
>
> greg k-h
Commit 6059f71f1e94 ("apparmor: add parameter to control whether policy
hashing is used") didn't even come in until 4.8 and SECURITY_APPARMOR_HASH_DEFAULT
isn't defined anywhere in 4.4. Fairly certain this isn't needed at all.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 21:56 ` Nathan Chancellor
@ 2018-09-17 22:12 ` John Johansen
2018-09-21 5:40 ` Loic
0 siblings, 1 reply; 16+ messages in thread
From: John Johansen @ 2018-09-17 22:12 UTC (permalink / raw)
To: Nathan Chancellor, Greg KH; +Cc: Loic, stable, arnd, james.l.morris
On 09/17/2018 02:56 PM, Nathan Chancellor wrote:
> On Mon, Sep 17, 2018 at 11:37:54PM +0200, Greg KH wrote:
>> On Mon, Sep 17, 2018 at 11:15:42PM +0200, Greg KH wrote:
>>> On Mon, Sep 17, 2018 at 09:45:47PM +0200, Loic wrote:
>>>> On Mon, 17 Sep 2018 15:58:56 +0200
>>>> Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>
>>>>> On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Tested without any problem so please picked up this for 4.4 to fix the
>>>>>> problem.
>>>>>> The patch below is slightly modified to adapt to this version.
>>>>>>
>>>>>> [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
>>>>>>
>>>>>> The newly added Kconfig option could never work and just causes a build
>>>>>> error
>>>>>> when disabled:
>>>>>>
>>>>>> security/apparmor/lsm.c:675:25: error:
>>>>>> 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
>>>>>> bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
>>>>>>
>>>>>> The problem is that the macro undefined in this case, and we need to use the
>>>>>> IS_ENABLED()
>>>>>> helper to turn it into a boolean constant.
>>>>>>
>>>>>> Another minor problem with the original patch is that the option is even
>>>>>> offered
>>>>>> in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the
>>>>>> option
>>>>>> in that case.
>>>>>>
>>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>>> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy
>>>>>> hashing is used")
>>>>>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>>>>>> Signed-off-by: James Morris <james.l.morris@oracle.com>
>>>>>> ---
>>>>>> diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
>>>>>> --- a/security/apparmor/crypto.c
>>>>>> +++ b/security/apparmor/crypto.c
>>>>>> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
>>>>>> int error = -ENOMEM;
>>>>>> u32 le32_version = cpu_to_le32(version);
>>>>>>
>>>>>> + if (!aa_g_hash_policy)
>>>>>> + return 0;
>>>>>> +
>>>>>> if (!apparmor_tfm)
>>>>>> return 0;
>>>>>>
>>>>>> diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>>>>>> --- a/security/apparmor/lsm.c
>>>>>> +++ b/security/apparmor/lsm.c
>>>>>> @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
>>>>>> module_param_call(mode, param_set_mode, param_get_mode,
>>>>>> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>>>>>>
>>>>>> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
>>>>>> +/* whether policy verification hashing is enabled */
>>>>>> +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
>>>>>> +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR |
>>>>>> S_IWUSR);
>>>>>> +#endif
>>>>>> +
>>>>>> /* Debug mode */
>>>>>> bool aa_g_debug;
>>>>>> module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
>>>>>> ---
>>>>>
>>>>> The patch is whitespace corrupted and can not be applied :(
>>>>
>>>> Sorry, I noticed the problem afterwards. I opened a bug report to try to fix my mail client:
>>>> https://github.com/roundcube/roundcubemail/issues/6438
>>>>
>>>>>
>>>>> Can you fix that up and resend it so that I can apply it?
>>>>
>>>> No problem. Thanks for all.
>>>>
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
>>>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>>>> Signed-off-by: James Morris <james.l.morris@oracle.com>
>>>> ---
>>>> diff -Nurp a/security/apparmor/crypto.c b/security/apparmor/crypto.c
>>>> --- a/security/apparmor/crypto.c
>>>> +++ b/security/apparmor/crypto.c
>>>> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
>>>> int error = -ENOMEM;
>>>> u32 le32_version = cpu_to_le32(version);
>>>>
>>>> + if (!aa_g_hash_policy)
>>>> + return 0;
>>>> +
>>>> if (!apparmor_tfm)
>>>> return 0;
>>>>
>>>> diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>>>> --- a/security/apparmor/lsm.c
>>>> +++ b/security/apparmor/lsm.c
>>>> @@ -692,6 +694,12 @@ enum profile_mode aa_g_profile_mode = AP
>>>> module_param_call(mode, param_set_mode, param_get_mode,
>>>> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>>>>
>>>> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
>>>> +/* whether policy verification hashing is enabled */
>>>> +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
>>>> +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
>>>> +#endif
>>>> +
>>>> /* Debug mode */
>>>> bool aa_g_debug;
>>>> module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
>>>
>>> THanks, that worked, now queued up.
>>
>> And dropped as this patch broke the build. I'm guessing this really
>> isn't needed if you didn't even test it worked :(
>>
>> Please fix all of this up, test it properly, and then send it as a
>> "clean" patch that I can apply easily.
>>
>> thanks,
>>
>> greg k-h
>
> Commit 6059f71f1e94 ("apparmor: add parameter to control whether policy
> hashing is used") didn't even come in until 4.8 and SECURITY_APPARMOR_HASH_DEFAULT
> isn't defined anywhere in 4.4. Fairly certain this isn't needed at all.
>
Sorry when I did my initial check on the patch backport deviating from
the upstream patch I didn't build test (assumed that had already been
done my bad), nor did I look into if it was valid (also my bad). I
should have.
I agree with your assessment, this isn't needed. 4.4 does not have
the kconfig nor code that uses it. The patch can be fixed, but
including it would be a feature backport not a bug fix like it was for
4.8 kernels.
Greg do not take this patch. I am sorry I wasted you time by missing
this on my initial check.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2018-09-17 22:12 ` John Johansen
@ 2018-09-21 5:40 ` Loic
0 siblings, 0 replies; 16+ messages in thread
From: Loic @ 2018-09-21 5:40 UTC (permalink / raw)
To: John Johansen; +Cc: Nathan Chancellor, Greg KH, stable, arnd, james.l.morris
Le 2018-09-18 00:12, John Johansen a écrit :
> On 09/17/2018 02:56 PM, Nathan Chancellor wrote:
>> On Mon, Sep 17, 2018 at 11:37:54PM +0200, Greg KH wrote:
>>> On Mon, Sep 17, 2018 at 11:15:42PM +0200, Greg KH wrote:
>>>> On Mon, Sep 17, 2018 at 09:45:47PM +0200, Loic wrote:
>>>>> On Mon, 17 Sep 2018 15:58:56 +0200
>>>>> Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>>
>>>>>> On Sun, Sep 09, 2018 at 04:04:18PM +0200, Loic wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> Tested without any problem so please picked up this for 4.4 to
>>>>>>> fix the
>>>>>>> problem.
>>>>>>> The patch below is slightly modified to adapt to this version.
>>>>>>>
>>>>>>> [ Upstream commit 7616ac70d1bb4f2e9d25c1a82d283f3368a7b632 ]
>>>>>>>
>>>>>>> The newly added Kconfig option could never work and just causes a
>>>>>>> build
>>>>>>> error
>>>>>>> when disabled:
>>>>>>>
>>>>>>> security/apparmor/lsm.c:675:25: error:
>>>>>>> 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a
>>>>>>> function)
>>>>>>> bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
>>>>>>>
>>>>>>> The problem is that the macro undefined in this case, and we need
>>>>>>> to use the
>>>>>>> IS_ENABLED()
>>>>>>> helper to turn it into a boolean constant.
>>>>>>>
>>>>>>> Another minor problem with the original patch is that the option
>>>>>>> is even
>>>>>>> offered
>>>>>>> in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also
>>>>>>> hides the
>>>>>>> option
>>>>>>> in that case.
>>>>>>>
>>>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>>>> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether
>>>>>>> policy
>>>>>>> hashing is used")
>>>>>>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>>>>>>> Signed-off-by: James Morris <james.l.morris@oracle.com>
>>>>>>> ---
>>>>>>> diff -Nurp a/security/apparmor/crypto.c
>>>>>>> b/security/apparmor/crypto.c
>>>>>>> --- a/security/apparmor/crypto.c
>>>>>>> +++ b/security/apparmor/crypto.c
>>>>>>> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
>>>>>>> int error = -ENOMEM;
>>>>>>> u32 le32_version = cpu_to_le32(version);
>>>>>>>
>>>>>>> + if (!aa_g_hash_policy)
>>>>>>> + return 0;
>>>>>>> +
>>>>>>> if (!apparmor_tfm)
>>>>>>> return 0;
>>>>>>>
>>>>>>> diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>>>>>>> --- a/security/apparmor/lsm.c
>>>>>>> +++ b/security/apparmor/lsm.c
>>>>>>> @@ -692,6 +692,12 @@ enum profile_mode aa_g_profile_mode = AP
>>>>>>> module_param_call(mode, param_set_mode, param_get_mode,
>>>>>>> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>>>>>>>
>>>>>>> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
>>>>>>> +/* whether policy verification hashing is enabled */
>>>>>>> +bool aa_g_hash_policy =
>>>>>>> IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
>>>>>>> +module_param_named(hash_policy, aa_g_hash_policy, aabool,
>>>>>>> S_IRUSR |
>>>>>>> S_IWUSR);
>>>>>>> +#endif
>>>>>>> +
>>>>>>> /* Debug mode */
>>>>>>> bool aa_g_debug;
>>>>>>> module_param_named(debug, aa_g_debug, aabool, S_IRUSR |
>>>>>>> S_IWUSR);
>>>>>>> ---
>>>>>>
>>>>>> The patch is whitespace corrupted and can not be applied :(
>>>>>
>>>>> Sorry, I noticed the problem afterwards. I opened a bug report to
>>>>> try to fix my mail client:
>>>>> https://github.com/roundcube/roundcubemail/issues/6438
>>>>>
>>>>>>
>>>>>> Can you fix that up and resend it so that I can apply it?
>>>>>
>>>>> No problem. Thanks for all.
>>>>>
>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether
>>>>> policy hashing is used")
>>>>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>>>>> Signed-off-by: James Morris <james.l.morris@oracle.com>
>>>>> ---
>>>>> diff -Nurp a/security/apparmor/crypto.c
>>>>> b/security/apparmor/crypto.c
>>>>> --- a/security/apparmor/crypto.c
>>>>> +++ b/security/apparmor/crypto.c
>>>>> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profi
>>>>> int error = -ENOMEM;
>>>>> u32 le32_version = cpu_to_le32(version);
>>>>>
>>>>> + if (!aa_g_hash_policy)
>>>>> + return 0;
>>>>> +
>>>>> if (!apparmor_tfm)
>>>>> return 0;
>>>>>
>>>>> diff -Nurp a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>>>>> --- a/security/apparmor/lsm.c
>>>>> +++ b/security/apparmor/lsm.c
>>>>> @@ -692,6 +694,12 @@ enum profile_mode aa_g_profile_mode = AP
>>>>> module_param_call(mode, param_set_mode, param_get_mode,
>>>>> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>>>>>
>>>>> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
>>>>> +/* whether policy verification hashing is enabled */
>>>>> +bool aa_g_hash_policy =
>>>>> IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
>>>>> +module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR
>>>>> | S_IWUSR);
>>>>> +#endif
>>>>> +
>>>>> /* Debug mode */
>>>>> bool aa_g_debug;
>>>>> module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
>>>>
>>>> THanks, that worked, now queued up.
>>>
>>> And dropped as this patch broke the build. I'm guessing this really
>>> isn't needed if you didn't even test it worked :(
>>>
>>> Please fix all of this up, test it properly, and then send it as a
>>> "clean" patch that I can apply easily.
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Commit 6059f71f1e94 ("apparmor: add parameter to control whether
>> policy
>> hashing is used") didn't even come in until 4.8 and
>> SECURITY_APPARMOR_HASH_DEFAULT
>> isn't defined anywhere in 4.4. Fairly certain this isn't needed at
>> all.
>>
>
> Sorry when I did my initial check on the patch backport deviating from
> the upstream patch I didn't build test (assumed that had already been
> done my bad), nor did I look into if it was valid (also my bad). I
> should have.
I'm the one following sorry, I forgot to report the Kconfig of the
latest version of the 4.9 kernel that I used during my tests. I admit my
mistakes and learn from them. Please excuse me!
> I agree with your assessment, this isn't needed. 4.4 does not have
> the kconfig nor code that uses it.
It's not really the backport functionality that's interesting but the
return 0.
Can you certify to me that "return 0" would be of no use?
> Greg do not take this patch. I am sorry I wasted you time by missing
> this on my initial check.
It's me, I'm going to review my method of submitting a patch.
--
Best regards,
Loic
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Patch 0/1] apparmor: fix to 4.8 pull request
@ 2016-07-25 17:59 John Johansen
2016-07-25 17:59 ` [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling John Johansen
0 siblings, 1 reply; 16+ messages in thread
From: John Johansen @ 2016-07-25 17:59 UTC (permalink / raw)
To: jmorris; +Cc: linux-kernel, linux-security-module
Hi James,
can you please pull the following bug fix to the 4.8 pull-request
The following changes since commit d4d03f74a73f3b8b2801d4d02011b6b69778cbcc:
apparmor: fix arg_size computation for when setprocattr is null terminated (2016-07-12 08:43:10 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor for-security
for you to fetch changes up to dc6f823283a3a97044655745cb5fd69749a0555a:
apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling (2016-07-25 10:44:59 -0700)
----------------------------------------------------------------
Arnd Bergmann (1):
apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
security/apparmor/crypto.c | 3 +++
security/apparmor/lsm.c | 4 +++-
security/apparmor/policy_unpack.c | 3 +--
3 files changed, 7 insertions(+), 3 deletions(-)
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2016-07-25 17:59 [Patch 0/1] apparmor: fix to 4.8 pull request John Johansen
@ 2016-07-25 17:59 ` John Johansen
2016-07-26 11:38 ` James Morris
0 siblings, 1 reply; 16+ messages in thread
From: John Johansen @ 2016-07-25 17:59 UTC (permalink / raw)
To: jmorris; +Cc: linux-kernel, linux-security-module, Arnd Bergmann
From: Arnd Bergmann <arnd@arndb.de>
The newly added Kconfig option could never work and just causes a build error
when disabled:
security/apparmor/lsm.c:675:25: error: 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
The problem is that the macro undefined in this case, and we need to use the IS_ENABLED()
helper to turn it into a boolean constant.
Another minor problem with the original patch is that the option is even offered
in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the option
in that case.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
security/apparmor/crypto.c | 3 +++
security/apparmor/lsm.c | 4 +++-
security/apparmor/policy_unpack.c | 3 +--
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
index 532471d..b75dab0 100644
--- a/security/apparmor/crypto.c
+++ b/security/apparmor/crypto.c
@@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
int error = -ENOMEM;
u32 le32_version = cpu_to_le32(version);
+ if (!aa_g_hash_policy)
+ return 0;
+
if (!apparmor_tfm)
return 0;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 3be30c7..41b8cb1 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -671,9 +671,11 @@ enum profile_mode aa_g_profile_mode = APPARMOR_ENFORCE;
module_param_call(mode, param_set_mode, param_get_mode,
&aa_g_profile_mode, S_IRUSR | S_IWUSR);
+#ifdef CONFIG_SECURITY_APPARMOR_HASH
/* whether policy verification hashing is enabled */
-bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
+bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
+#endif
/* Debug mode */
bool aa_g_debug;
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index b9b1c66..1381206 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -778,8 +778,7 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns)
if (error)
goto fail_profile;
- if (aa_g_hash_policy)
- error = aa_calc_profile_hash(profile, e.version, start,
+ error = aa_calc_profile_hash(profile, e.version, start,
e.pos - start);
if (error)
goto fail_profile;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2016-07-25 17:59 ` [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling John Johansen
@ 2016-07-26 11:38 ` James Morris
2016-07-26 16:56 ` John Johansen
0 siblings, 1 reply; 16+ messages in thread
From: James Morris @ 2016-07-26 11:38 UTC (permalink / raw)
To: John Johansen; +Cc: linux-kernel, linux-security-module, Arnd Bergmann
On Mon, 25 Jul 2016, John Johansen wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly added Kconfig option could never work and just causes a build error
> when disabled:
>
> security/apparmor/lsm.c:675:25: error: 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
>
> The problem is that the macro undefined in this case, and we need to use the IS_ENABLED()
> helper to turn it into a boolean constant.
>
> Another minor problem with the original patch is that the option is even offered
> in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the option
> in that case.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
> Signed-off-by: John Johansen <john.johansen@canonical.com>
Please rebase this against current Linus.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2016-07-26 11:38 ` James Morris
@ 2016-07-26 16:56 ` John Johansen
0 siblings, 0 replies; 16+ messages in thread
From: John Johansen @ 2016-07-26 16:56 UTC (permalink / raw)
To: James Morris; +Cc: linux-kernel, linux-security-module, Arnd Bergmann
On 07/26/2016 04:38 AM, James Morris wrote:
> On Mon, 25 Jul 2016, John Johansen wrote:
>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The newly added Kconfig option could never work and just causes a build error
>> when disabled:
>>
>> security/apparmor/lsm.c:675:25: error: 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
>> bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
>>
>> The problem is that the macro undefined in this case, and we need to use the IS_ENABLED()
>> helper to turn it into a boolean constant.
>>
>> Another minor problem with the original patch is that the option is even offered
>> in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the option
>> in that case.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>
> Please rebase this against current Linus.
>
Well this is a fix for a patch in the security next queue, so linus doesn't
have it yet. I can wait for your 4.8 pull request and rebase then
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
@ 2016-07-13 20:50 Arnd Bergmann
2016-07-13 21:14 ` John Johansen
0 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-07-13 20:50 UTC (permalink / raw)
To: John Johansen
Cc: Arnd Bergmann, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel
The newly added Kconfig option could never work and just causes a build error
when disabled:
security/apparmor/lsm.c:675:25: error: 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
The problem is that the macro undefined in this case, and we need to use the IS_ENABLED()
helper to turn it into a boolean constant.
Another minor problem with the original patch is that the option is even offered
in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the option
in that case.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
---
security/apparmor/crypto.c | 3 +++
security/apparmor/lsm.c | 4 +++-
security/apparmor/policy_unpack.c | 3 +--
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
index 532471d0b3a0..b75dab0df1cb 100644
--- a/security/apparmor/crypto.c
+++ b/security/apparmor/crypto.c
@@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
int error = -ENOMEM;
u32 le32_version = cpu_to_le32(version);
+ if (!aa_g_hash_policy)
+ return 0;
+
if (!apparmor_tfm)
return 0;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 3be30c701bfa..41b8cb115801 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -671,9 +671,11 @@ enum profile_mode aa_g_profile_mode = APPARMOR_ENFORCE;
module_param_call(mode, param_set_mode, param_get_mode,
&aa_g_profile_mode, S_IRUSR | S_IWUSR);
+#ifdef CONFIG_SECURITY_APPARMOR_HASH
/* whether policy verification hashing is enabled */
-bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
+bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
+#endif
/* Debug mode */
bool aa_g_debug;
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index b9b1c66a32a5..138120698f83 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -778,8 +778,7 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns)
if (error)
goto fail_profile;
- if (aa_g_hash_policy)
- error = aa_calc_profile_hash(profile, e.version, start,
+ error = aa_calc_profile_hash(profile, e.version, start,
e.pos - start);
if (error)
goto fail_profile;
--
2.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling
2016-07-13 20:50 Arnd Bergmann
@ 2016-07-13 21:14 ` John Johansen
0 siblings, 0 replies; 16+ messages in thread
From: John Johansen @ 2016-07-13 21:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel
On 07/13/2016 01:50 PM, Arnd Bergmann wrote:
> The newly added Kconfig option could never work and just causes a build error
> when disabled:
>
> security/apparmor/lsm.c:675:25: error: 'CONFIG_SECURITY_APPARMOR_HASH_DEFAULT' undeclared here (not in a function)
> bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
>
> The problem is that the macro undefined in this case, and we need to use the IS_ENABLED()
> helper to turn it into a boolean constant.
>
oops yep, looks like I've built with Y and N but not left undefined
> Another minor problem with the original patch is that the option is even offered
> in sysfs when SECURITY_APPARMOR_HASH is not enabled, so this also hides the option
> in that case.
>
ah yeah nice
also nice that you pulled the condition check into the calc_profile_hash() fn so the
check won't get accidentally dropped by something else calling it
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: John Johansen <john.johansen@canonical.com>
> Fixes: 6059f71f1e94 ("apparmor: add parameter to control whether policy hashing is used")
> ---
> security/apparmor/crypto.c | 3 +++
> security/apparmor/lsm.c | 4 +++-
> security/apparmor/policy_unpack.c | 3 +--
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> index 532471d0b3a0..b75dab0df1cb 100644
> --- a/security/apparmor/crypto.c
> +++ b/security/apparmor/crypto.c
> @@ -39,6 +39,9 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
> int error = -ENOMEM;
> u32 le32_version = cpu_to_le32(version);
>
> + if (!aa_g_hash_policy)
> + return 0;
> +
> if (!apparmor_tfm)
> return 0;
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 3be30c701bfa..41b8cb115801 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -671,9 +671,11 @@ enum profile_mode aa_g_profile_mode = APPARMOR_ENFORCE;
> module_param_call(mode, param_set_mode, param_get_mode,
> &aa_g_profile_mode, S_IRUSR | S_IWUSR);
>
> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> /* whether policy verification hashing is enabled */
> -bool aa_g_hash_policy = CONFIG_SECURITY_APPARMOR_HASH_DEFAULT;
> +bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
> module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
> +#endif
>
> /* Debug mode */
> bool aa_g_debug;
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index b9b1c66a32a5..138120698f83 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -778,8 +778,7 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns)
> if (error)
> goto fail_profile;
>
> - if (aa_g_hash_policy)
> - error = aa_calc_profile_hash(profile, e.version, start,
> + error = aa_calc_profile_hash(profile, e.version, start,
> e.pos - start);
> if (error)
> goto fail_profile;
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-09-21 11:36 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-09 14:04 [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling Loic
2018-09-17 11:49 ` Greg KH
2018-09-17 13:40 ` John Johansen
2018-09-17 13:58 ` Greg KH
2018-09-17 13:58 ` Greg KH
2018-09-17 19:45 ` Loic
2018-09-17 21:15 ` Greg KH
2018-09-17 21:37 ` Greg KH
2018-09-17 21:56 ` Nathan Chancellor
2018-09-17 22:12 ` John Johansen
2018-09-21 5:40 ` Loic
-- strict thread matches above, loose matches on Subject: below --
2016-07-25 17:59 [Patch 0/1] apparmor: fix to 4.8 pull request John Johansen
2016-07-25 17:59 ` [PATCH] apparmor: fix SECURITY_APPARMOR_HASH_DEFAULT parameter handling John Johansen
2016-07-26 11:38 ` James Morris
2016-07-26 16:56 ` John Johansen
2016-07-13 20:50 Arnd Bergmann
2016-07-13 21:14 ` John Johansen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.