nr_in_ready_table and nr_in_build_table can underflow in if statement

Bug #1854054 reported by Dan Streetman on 2019-11-26
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
cachefilesd (Debian)
New
Unknown
cachefilesd (Ubuntu)
Status tracked in Focal
Xenial
Undecided
Unassigned
Bionic
Medium
Dan Streetman
Disco
Medium
Dan Streetman
Eoan
Medium
Dan Streetman
Focal
Medium
Dan Streetman

Bug Description

[impact]

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.

[test case]

this is difficult to reproduce and it's unclear the specific conditions that can reproduce it, but it has been reported to happen and review of the code shows it clearly could happen.

[regression potential]

this simply moves the -1 over to the for loop counter as a +1, so the most likely regression would be a for loop counter overflow. However that should not happen as the culltable_size is limited to 4096, and the for loop counter is unsigned int; so it should be safe from overflow. Any other regression would likely involve a similar result as the current bug, a segfault.

[other info]

this bug does not exist in Xenial, as the counters there are signed ints, so underflow (from 0) does not happen.

Dan Streetman (ddstreet) on 2019-11-26
Changed in cachefilesd (Ubuntu Xenial):
status: New → Invalid
Changed in cachefilesd (Ubuntu Bionic):
status: New → Triaged
status: Triaged → In Progress
Changed in cachefilesd (Ubuntu Disco):
status: New → In Progress
Changed in cachefilesd (Ubuntu Eoan):
status: New → In Progress
Changed in cachefilesd (Ubuntu Focal):
status: New → In Progress
assignee: nobody → Dan Streetman (ddstreet)
Changed in cachefilesd (Ubuntu Eoan):
assignee: nobody → Dan Streetman (ddstreet)
Changed in cachefilesd (Ubuntu Disco):
assignee: nobody → Dan Streetman (ddstreet)
Changed in cachefilesd (Ubuntu Bionic):
assignee: nobody → Dan Streetman (ddstreet)
Changed in cachefilesd (Ubuntu Focal):
importance: Undecided → Low
Changed in cachefilesd (Ubuntu Eoan):
importance: Undecided → Low
Changed in cachefilesd (Ubuntu Focal):
importance: Low → Medium
Changed in cachefilesd (Ubuntu Eoan):
importance: Low → Medium
Changed in cachefilesd (Ubuntu Disco):
importance: Undecided → Medium
Changed in cachefilesd (Ubuntu Bionic):
importance: Undecided → Medium
description: updated
Changed in cachefilesd (Debian):
status: Unknown → New
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cachefilesd - 0.10.10-0.2ubuntu1

---------------
cachefilesd (0.10.10-0.2ubuntu1) focal; urgency=medium

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

 -- Dan Streetman <email address hidden> Tue, 26 Nov 2019 08:10:43 -0500

Changed in cachefilesd (Ubuntu Focal):
status: In Progress → Fix Released

Hello Dan, or anyone else affected,

Accepted cachefilesd into eoan-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cachefilesd/0.10.10-0.2ubuntu0.19.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-eoan to verification-done-eoan. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-eoan. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in cachefilesd (Ubuntu Eoan):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-eoan
Timo Aaltonen (tjaalton) wrote :

Hello Dan, or anyone else affected,

Accepted cachefilesd into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cachefilesd/0.10.10-0.1ubuntu0.19.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in cachefilesd (Ubuntu Disco):
status: In Progress → Fix Committed
tags: added: verification-needed-disco
Changed in cachefilesd (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed-bionic
Timo Aaltonen (tjaalton) wrote :

Hello Dan, or anyone else affected,

Accepted cachefilesd into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cachefilesd/0.10.10-0.1ubuntu0.18.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Dan Streetman (ddstreet) wrote :

status note - I'm waiting on the original reporter to provide results of testing to me, I'll update verification status once I hear from them.

Dan Streetman (ddstreet) wrote :

status note - still waiting for the original reporter to provide testing results to verify this bug; if I haven't got verification from them after I get back from the holidays, I may see if i can set up some kind of verifier for this so we can speed up its release.

Dan Streetman (ddstreet) wrote :

reporter has indicated the problem was not hit in the last several weeks using the patched package, so marking this bug as verified.

tags: added: verification-done verification-done-bionic verification-done-disco verification-done-eoan
removed: verification-needed verification-needed-bionic verification-needed-disco verification-needed-eoan
Łukasz Zemczak (sil2100) wrote :

I think it was aging in -proposed for a sufficient time. Releasing.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cachefilesd - 0.10.10-0.2ubuntu0.19.10.1

---------------
cachefilesd (0.10.10-0.2ubuntu0.19.10.1) eoan; urgency=medium

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

 -- Dan Streetman <email address hidden> Tue, 26 Nov 2019 08:16:09 -0500

Changed in cachefilesd (Ubuntu Eoan):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for cachefilesd has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cachefilesd - 0.10.10-0.1ubuntu0.18.04.1

---------------
cachefilesd (0.10.10-0.1ubuntu0.18.04.1) bionic; urgency=medium

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

 -- Dan Streetman <email address hidden> Tue, 26 Nov 2019 08:01:23 -0500

Changed in cachefilesd (Ubuntu Bionic):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cachefilesd - 0.10.10-0.1ubuntu0.19.04.1

---------------
cachefilesd (0.10.10-0.1ubuntu0.19.04.1) disco; urgency=medium

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

 -- Dan Streetman <email address hidden> Tue, 26 Nov 2019 08:18:03 -0500

Changed in cachefilesd (Ubuntu Disco):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.