[SRU][T/A][PATCH 0/1] Fix for CVE-2018-1094

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][T/A][PATCH 0/1] Fix for CVE-2018-1094

Khalid Elmously
The CVE info page states that 2 upstream patches are required to fix this CVE, however I think that's incorrect and I believe only 1 patch is needed for the CVE which is a45403b51582a87872927a3e0fc0a389c26867f1

Also the matrix states that xenial is vulnerable to this CVE, however I don't think that's true either (it already has a45403b51582a87872927a3e0fc0a389c26867f1 ).

Clean cherry-pick for artful and a straight-forward backport for trusty.


Theodore Ts'o (1):
  ext4: always initialize the crc32c checksum driver

 fs/ext4/super.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][T][PATCH 1/1] ext4: always initialize the crc32c checksum driver

Khalid Elmously
From: Theodore Ts'o <[hidden email]>

CVE-2018-1094

The extended attribute code now uses the crc32c checksum for hashing
purposes, so we should just always always initialize it.  We also want
to prevent NULL pointer dereferences if one of the metadata checksum
features is enabled after the file sytsem is originally mounted.

This issue has been assigned CVE-2018-1094.

https://bugzilla.kernel.org/show_bug.cgi?id=199183
https://bugzilla.redhat.com/show_bug.cgi?id=1560788

Signed-off-by: Theodore Ts'o <[hidden email]>
Cc: [hidden email]
(backported from a45403b51582a87872927a3e0fc0a389c26867f1)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 fs/ext4/super.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3b313429b83f..26552bf46ca8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3457,15 +3457,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
  }
 
  /* Load the checksum driver */
- if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
-       EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
- sbi->s_chksum_driver = crypto_alloc_shash("crc32c", 0, 0);
- if (IS_ERR(sbi->s_chksum_driver)) {
- ext4_msg(sb, KERN_ERR, "Cannot load crc32c driver.");
- ret = PTR_ERR(sbi->s_chksum_driver);
- sbi->s_chksum_driver = NULL;
- goto failed_mount;
- }
+ sbi->s_chksum_driver = crypto_alloc_shash("crc32c", 0, 0);
+ if (IS_ERR(sbi->s_chksum_driver)) {
+ ext4_msg(sb, KERN_ERR, "Cannot load crc32c driver.");
+ ret = PTR_ERR(sbi->s_chksum_driver);
+ sbi->s_chksum_driver = NULL;
+ goto failed_mount;
  }
 
  /* Check superblock checksum */
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][A][PATCH 1/1] ext4: always initialize the crc32c checksum driver

Khalid Elmously
In reply to this post by Khalid Elmously
From: Theodore Ts'o <[hidden email]>

CVE-2018-1094

The extended attribute code now uses the crc32c checksum for hashing
purposes, so we should just always always initialize it.  We also want
to prevent NULL pointer dereferences if one of the metadata checksum
features is enabled after the file sytsem is originally mounted.

This issue has been assigned CVE-2018-1094.

https://bugzilla.kernel.org/show_bug.cgi?id=199183
https://bugzilla.redhat.com/show_bug.cgi?id=1560788

Signed-off-by: Theodore Ts'o <[hidden email]>
Cc: [hidden email]
(cherry-picked from a45403b51582a87872927a3e0fc0a389c26867f1)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 fs/ext4/super.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 392bc1f88f21..24521c830617 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3518,15 +3518,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
  }
 
  /* Load the checksum driver */
- if (ext4_has_feature_metadata_csum(sb) ||
-    ext4_has_feature_ea_inode(sb)) {
- sbi->s_chksum_driver = crypto_alloc_shash("crc32c", 0, 0);
- if (IS_ERR(sbi->s_chksum_driver)) {
- ext4_msg(sb, KERN_ERR, "Cannot load crc32c driver.");
- ret = PTR_ERR(sbi->s_chksum_driver);
- sbi->s_chksum_driver = NULL;
- goto failed_mount;
- }
+ sbi->s_chksum_driver = crypto_alloc_shash("crc32c", 0, 0);
+ if (IS_ERR(sbi->s_chksum_driver)) {
+ ext4_msg(sb, KERN_ERR, "Cannot load crc32c driver.");
+ ret = PTR_ERR(sbi->s_chksum_driver);
+ sbi->s_chksum_driver = NULL;
+ goto failed_mount;
  }
 
  /* Check superblock checksum */
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK/cmnt: [SRU][T/A][PATCH 0/1] Fix for CVE-2018-1094

Stefan Bader-2
In reply to this post by Khalid Elmously
On 06.07.2018 23:02, Khalid Elmously wrote:

> The CVE info page states that 2 upstream patches are required to fix this CVE, however I think that's incorrect and I believe only 1 patch is needed for the CVE which is a45403b51582a87872927a3e0fc0a389c26867f1
>
> Also the matrix states that xenial is vulnerable to this CVE, however I don't think that's true either (it already has a45403b51582a87872927a3e0fc0a389c26867f1 ).
>
> Clean cherry-pick for artful and a straight-forward backport for trusty.
>
>
> Theodore Ts'o (1):
>   ext4: always initialize the crc32c checksum driver
>
>  fs/ext4/super.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
I agree that the second patch seems to have nothing to do with the description
of the CVE. Maybe this is another CVE which slipped in.

For Artful, the NACK is because its not high/critical and that release is EOL
(or close).

For Xenial and Trusty, I believe from the description text that the fix may
refer to this 4.13 patch that does introduce some crc32_hash variable:

commit b9fc761ea2d82e910e92f83d01bbbbe1f5e99bfc
Author: Tahsin Erdogan <[hidden email]>
Date:   Thu Jun 22 11:53:15 2017 -0400

    ext4: strong binding of xattr inode references

So I would assume, that before that the crc32 driver would be needed only
depending on the features which are checked. And from that kernels before 4.13
to be not affected (whould maybe be supported by the fact that this simple
change never got into 4.4.y upstream).

If this is wrong, then still it looks like the second patch should be paired with:

commit 7ef79ad52136712172eb0525bf0b462516bf2f93
Author: Theodore Ts'o <[hidden email]>
Date:   Thu Apr 26 00:44:46 2018 -0400

    ext4: add MODULE_SOFTDEP to ensure crc32c is included in the initramfs

    Fixes: a45403b51582 ("ext4: always initialize the crc32c checksum driver")
    Reported-by: Fran├žois Valenduc <[hidden email]>
    Signed-off-by: Theodore Ts'o <[hidden email]>
    Cc: [hidden email]

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment