1. Patchew
  2. linux
  3. View series

[PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg

Coiby Xu posted 1 patch 1 year, 11 months ago 
security/integrity/digsig.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Coiby Xu 1 year, 11 months ago 
Currently when the kernel fails to add a cert to the .machine keyring,
it will throw an error immediately in the function integrity_add_key.

Since the kernel will try adding to the .platform keyring next or throw
an error (in the caller of integrity_add_key i.e. add_to_machine_keyring),
so there is no need to throw an error immediately in integrity_add_key.

Reported-by: itrymybest80@protonmail.com
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331
Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine")
Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
v2
 - improve patch subject [Mimi]
 - add Fixes tag [Jarkko]
 - add Reviewed-by tag from Eric
---
 security/integrity/digsig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index df387de29bfa..45c3e5dda355 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -179,7 +179,8 @@ static int __init integrity_add_key(const unsigned int id, const void *data,
 				   KEY_ALLOC_NOT_IN_QUOTA);
 	if (IS_ERR(key)) {
 		rc = PTR_ERR(key);
-		pr_err("Problem loading X.509 certificate %d\n", rc);
+		if (id != INTEGRITY_KEYRING_MACHINE)
+			pr_err("Problem loading X.509 certificate %d\n", rc);
 	} else {
 		pr_notice("Loaded X.509 cert '%s'\n",
 			  key_ref_to_ptr(key)->description);
-- 
2.43.0
Re: [PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Maxime Ripard 1 year, 9 months ago 
Hi Dmitry, Eric, James, Mimi, Paul, Serge,

On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote:
> Currently when the kernel fails to add a cert to the .machine keyring,
> it will throw an error immediately in the function integrity_add_key.
> 
> Since the kernel will try adding to the .platform keyring next or throw
> an error (in the caller of integrity_add_key i.e. add_to_machine_keyring),
> so there is no need to throw an error immediately in integrity_add_key.
> 
> Reported-by: itrymybest80@protonmail.com
> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331
> Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine")
> Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>

Any chance this patch can be merged? This is breaking (at least) Fedora
at the moment.

Thanks!
Maxime
Re: [PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Mimi Zohar 1 year, 9 months ago 
On Wed, 2024-03-06 at 11:57 +0100, Maxime Ripard wrote:
> Hi Dmitry, Eric, James, Mimi, Paul, Serge,
> 
> On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote:
> > Currently when the kernel fails to add a cert to the .machine keyring,
> > it will throw an error immediately in the function integrity_add_key.
> > 
> > Since the kernel will try adding to the .platform keyring next or throw
> > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring),
> > so there is no need to throw an error immediately in integrity_add_key.
> > 
> > Reported-by: itrymybest80@protonmail.com
> > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331
> > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine")
> > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
> > Signed-off-by: Coiby Xu <coxu@redhat.com>
> 
> Any chance this patch can be merged? This is breaking (at least) Fedora
> at the moment.

https://git.kernel.org/torvalds/c/29cd507cbec282e13dcf8f38072a100af96b2bb7

Mimi
Re: [PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Maxime Ripard 1 year, 9 months ago 
On Wed, Mar 06, 2024 at 06:55:00AM -0500, Mimi Zohar wrote:
> On Wed, 2024-03-06 at 11:57 +0100, Maxime Ripard wrote:
> > Hi Dmitry, Eric, James, Mimi, Paul, Serge,
> > 
> > On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote:
> > > Currently when the kernel fails to add a cert to the .machine keyring,
> > > it will throw an error immediately in the function integrity_add_key.
> > > 
> > > Since the kernel will try adding to the .platform keyring next or throw
> > > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring),
> > > so there is no need to throw an error immediately in integrity_add_key.
> > > 
> > > Reported-by: itrymybest80@protonmail.com
> > > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331
> > > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine")
> > > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > 
> > Any chance this patch can be merged? This is breaking (at least) Fedora
> > at the moment.
> 
> https://git.kernel.org/torvalds/c/29cd507cbec282e13dcf8f38072a100af96b2bb7

Oh, awesome, we missed it.

Thanks!
Maxime
Re: [PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Coiby Xu 1 year, 9 months ago 
On Wed, Mar 06, 2024 at 01:40:01PM +0100, Maxime Ripard wrote:
>On Wed, Mar 06, 2024 at 06:55:00AM -0500, Mimi Zohar wrote:
>> On Wed, 2024-03-06 at 11:57 +0100, Maxime Ripard wrote:
>> > Hi Dmitry, Eric, James, Mimi, Paul, Serge,
>> >
>> > On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote:
>> > > Currently when the kernel fails to add a cert to the .machine keyring,
>> > > it will throw an error immediately in the function integrity_add_key.
>> > >
>> > > Since the kernel will try adding to the .platform keyring next or throw
>> > > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring),
>> > > so there is no need to throw an error immediately in integrity_add_key.
>> > >
>> > > Reported-by: itrymybest80@protonmail.com
>> > > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331
>> > > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine")
>> > > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
>> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> >
>> > Any chance this patch can be merged? This is breaking (at least) Fedora
>> > at the moment.
>>
>> https://git.kernel.org/torvalds/c/29cd507cbec282e13dcf8f38072a100af96b2bb7
>
>Oh, awesome, we missed it.

Oh, I missed the emails about Mimi's PR sent to Linus as well. Btw, I'm
curious to ask why you used the word "breaking" because I thought these KERN_ERR
errors shouldn't cause any real problem.

-- 
Best regards,
Coiby
Re: [PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Coiby Xu 1 year, 10 months ago 
Hi Mimi,

Could you take a look at this version of patch? If it escaped your
attention because it got buried in the same thread, sorry for that. And
I won't send new version as a reply to previous version in the future.

On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote:
>Currently when the kernel fails to add a cert to the .machine keyring,
>it will throw an error immediately in the function integrity_add_key.
>
>Since the kernel will try adding to the .platform keyring next or throw
>an error (in the caller of integrity_add_key i.e. add_to_machine_keyring),
>so there is no need to throw an error immediately in integrity_add_key.
>
>Reported-by: itrymybest80@protonmail.com
>Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331
>Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine")
>Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
>Signed-off-by: Coiby Xu <coxu@redhat.com>
>---
>v2
> - improve patch subject [Mimi]
> - add Fixes tag [Jarkko]
> - add Reviewed-by tag from Eric
>---
> security/integrity/digsig.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
>index df387de29bfa..45c3e5dda355 100644
>--- a/security/integrity/digsig.c
>+++ b/security/integrity/digsig.c
>@@ -179,7 +179,8 @@ static int __init integrity_add_key(const unsigned int id, const void *data,
> 				   KEY_ALLOC_NOT_IN_QUOTA);
> 	if (IS_ERR(key)) {
> 		rc = PTR_ERR(key);
>-		pr_err("Problem loading X.509 certificate %d\n", rc);
>+		if (id != INTEGRITY_KEYRING_MACHINE)
>+			pr_err("Problem loading X.509 certificate %d\n", rc);
> 	} else {
> 		pr_notice("Loaded X.509 cert '%s'\n",
> 			  key_ref_to_ptr(key)->description);
>-- 
>2.43.0
>

-- 
Best regards,
Coiby
Re: [PATCH v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg
Posted by Mimi Zohar 1 year, 10 months ago 
On Fri, 2024-02-16 at 19:10 +0800, Coiby Xu wrote:
> Hi Mimi,
> 
> Could you take a look at this version of patch? If it escaped your
> attention because it got buried in the same thread, sorry for that. And
> I won't send new version as a reply to previous version in the future.

Thanks for the reminder.

Mimi

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.