ACK: [SRU][B/hwe][CVE-2020-14314][PATCH 0/1] CVE-2020-14314 fix

[Andrea Righi]

On Wed, Nov 04, 2020 at 05:59:15PM -0700, Kelsey Skunberg wrote:
> CVE-2020-14314
> 
> SRU Justification:
> 
> [Impact]
> 
> A memory out-of-bounds read flaw was found in the Linux kernel before
> 5.9-rc2 with the ext3/ext4 file system, in the way it accesses a
> directory with broken indexing. This flaw allows a local user to crash
> the system if the directory exists. The highest threat from this
> vulnerability is to system availability.
> 
> Upstream cover-letter for patch from Eric Sandeen:
> 
> "We recently had a report of a panic in do_split; the filesystem in
> question panicked a distribution kernel when trying to add a new
> directory entry; the behavior/bug persists upstream.
> 
> The directory block in question had lots of unused and un-coalesced
> entries, like this, printed from the loop in ext4_insert_dentry():
> 
> [32778.024654] reclen 44 for name len 36
> [32778.028745] start: de ffff9f4cb5309800 top ffff9f4cb5309bd4
> [32778.034971]  offset 0 nlen 28 rlen 40, rlen-nlen 12, reclen 44 name <empty>
> [32778.042744]  offset 40 nlen 28 rlen 28, rlen-nlen 0, reclen 44 name <empty>
> [32778.050521]  offset 68 nlen 32 rlen 32, rlen-nlen 0, reclen 44 name <empty>
> [32778.058294]  offset 100 nlen 28 rlen 28, rlen-nlen 0, reclen 44 name <empty>
> [32778.066166]  offset 128 nlen 28 rlen 28, rlen-nlen 0, reclen 44 name <empty>
> [32778.074035]  offset 156 nlen 28 rlen 28, rlen-nlen 0, reclen 44 name <empty>
> [32778.081907]  offset 184 nlen 24 rlen 24, rlen-nlen 0, reclen 44 name <empty>
> [32778.089779]  offset 208 nlen 36 rlen 36, rlen-nlen 0, reclen 44 name <empty>
> [32778.097648]  offset 244 nlen 12 rlen 12, rlen-nlen 0, reclen 44 name REDACTED
> [32778.105227]  offset 256 nlen 24 rlen 24, rlen-nlen 0, reclen 44 name <empty>
> [32778.113099]  offset 280 nlen 24 rlen 24, rlen-nlen 0, reclen 44 name REDACTED
> [32778.122134]  offset 304 nlen 20 rlen 20, rlen-nlen 0, reclen 44 name REDACTED
> [32778.130780]  offset 324 nlen 16 rlen 16, rlen-nlen 0, reclen 44 name REDACTED
> [32778.138746]  offset 340 nlen 24 rlen 24, rlen-nlen 0, reclen 44 name <empty>
> [32778.146616]  offset 364 nlen 28 rlen 28, rlen-nlen 0, reclen 44 name <empty>
> [32778.154487]  offset 392 nlen 24 rlen 24, rlen-nlen 0, reclen 44 name <empty>
> [32778.162362]  offset 416 nlen 16 rlen 16, rlen-nlen 0, reclen 44 name <empty>
> ...
> 
> the file we were trying to insert needed a record length of 44, and none
> of the non-coalesced <empty> slots were big enough, so we failed and
> told do_split to get to work.
> 
> However, the sum of the non-empty entries didn't exceed half the block
> size, so the loop in do_split() iterated over all of the entries, ended
> at "count," and told us to split at (count - move) which is zero, and
> eventually:
> 
>         continued = hash2 == map[split - 1].hash;
> 
> exploded on the negative index.
> 
> It's an open question as to how this directory got into this format; I'm
> not sure if this should ever happen or not.  But at a minimum, I think
> we should be defensive here, hence [PATCH 1/1] will do that as an
> expedient fix and backportable patch for this situation.  There may be
> some other underlying probem which led to this directory structure if
> it's unexpected, and maybe that can come as another patch if anyone can
> investigate."
> 
> [Fix]
> 
> Apply following patch:
> 
> 5872331b3d91 ("ext4: fix potential negative array index in do_split()")
> 
> [Test Case]
> 
> Example of when the failure hits is listed above. Reproducer was not
> provided and as mentioned above, it's unknown how the directory got into
> that format. Patch was submitted as a defensive measure to avoid
> risk of using a negative index.
> 
> [Regression Potential]
> 
> Minimal regression risk. Only risk seen is if the split blocks do not
> have adequate spacing after the split. However, if 'split = count/2' is
> ran, the number of active entries is less than half the size of the
> block. In this case, there should still be plenty of space (> half
> blocksize) in each split block.
> 
> [Other]
> 
> Already in Groovy, Focal, Bionic, Xenial

Clean upstream cherry-pick, very good coverage of the issue, looks good
to me, thanks!

Acked-by: Andrea Righi <andrea.righi at canonical.com>