Bug#945527: cachefilesd: if statement nr_in_(ready|build)_table underflow causes segfault

[Dan Streetman]

On Wed, Jun 3, 2020 at 5:39 AM Daniel Scharon
 wrote:
>
> Dear Maintainer,
>
> are you willing to accept this patch?
>
> @Dan: Is your patch or at least the bug itself reported to upstream?

Upstream cachefilesd hasn't seen any commits in over 3 years:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/cachefilesd.git

and the maintainer says he plans to 'overhaul the whole thing':
https://www.redhat.com/archives/linux-cachefs/2020-March/msg2.html

so I considered upstream essentially dead.

>
> Thank you and kind regards,
> Daniel

Bug#945527: cachefilesd: if statement nr_in_(ready|build)_table underflow causes segfault

[Daniel Scharon]

Dear Maintainer,

are you willing to accept this patch?

@Dan: Is your patch or at least the bug itself reported to upstream?

Thank you and kind regards,
Daniel


smime.p7s
Description: S/MIME cryptographic signature

Bug#945527: cachefilesd: if statement nr_in_(ready|build)_table underflow causes segfault

[Dan Streetman]

Package: cachefilesd
Version: 0.10.10-0.2
Severity: normal
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu focal ubuntu-patch

Dear Maintainer,

the build_cull_table() function scans through elements up to 
nr_in_ready_table/nr_in_build_table, and performs actions if a match was found; 
however the match detection logic simply compares the for loop index against 
nr_in_*_table - 1, which underflows when 0, resulting in incorrect if section 
being run and then segfaulting.


In Ubuntu, the attached patch was applied to achieve the following:

  * Avoid counter underflow, leading to segfault (LP: #1854054)


Thanks for considering the patch.


-- System Information:
Debian Release: buster/sid
  APT prefers eoan-updates
  APT policy: (500, 'eoan-updates'), (500, 'eoan-security'), (500, 'eoan'), 
(100, 'eoan-backports')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.3.0-23-generic (SMP w/24 CPU cores)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US 
(charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
diff -Nru cachefilesd-0.10.10/cachefilesd.c cachefilesd-0.10.10/cachefilesd.c
--- cachefilesd-0.10.10/cachefilesd.c   2019-07-19 10:55:33.0 -0400
+++ cachefilesd-0.10.10/cachefilesd.c   2019-11-26 08:09:57.0 -0500
@@ -1389,13 +1389,13 @@
if (cullready[loop] == child)
break;
 
-   if (loop == nr_in_ready_table - 1) {
+   if (loop + 1 == nr_in_ready_table) {
/* child was oldest object */
cullready[--nr_in_ready_table] = (void 
*)(0x6b00 | __LINE__);
put_object(child);
goto removed;
}
-   else if (loop < nr_in_ready_table - 1) {
+   else if (loop + 1 < nr_in_ready_table) {
/* child was somewhere in between */
memmove([loop],
[loop + 1],
@@ -1409,12 +1409,12 @@
if (cullbuild[loop] == child)
break;
 
-   if (loop == nr_in_build_table - 1) {
+   if (loop + 1 == nr_in_build_table) {
/* child was oldest object */
cullbuild[--nr_in_build_table] = (void 
*)(0x6b00 | __LINE__);
put_object(child);
}
-   else if (loop < nr_in_build_table - 1) {
+   else if (loop + 1 < nr_in_build_table) {
/* child was somewhere in between */
memmove([loop],
[loop + 1],