Skip to content

Fix use-after-free deleting an in-use printer. - #1722

Closed
andreer wants to merge 1 commit into
OpenPrinting:masterfrom
andreer:fix-cupsd-delete-uaf
Closed

andreer wants to merge 1 commit into
OpenPrinting:masterfrom
andreer:fix-cupsd-delete-uaf

Conversation

@andreer

@andreer andreer commented Sep 26, 2026

Copy link
Copy Markdown

create_local_bg_thread() holds a reference to the printer via printer->use while it generates the IPP Everywhere PPD. cupsdDeleteTemporaryPrinters() respected that reference, but cupsdDeletePrinter() did not, so an explicit CUPS-Delete-Printer (as cups-browsed issues when it replaces a discovered queue) freed the printer mid-thread and corrupted the heap ("corrupted double-linked list").

Defer deletion while printer->use > 0 by flagging the printer under printer->lock; cupsdDeleteTemporaryPrinters() then reaps it once the thread releases its reference. Extends the use-count protection from #1655 to the explicit deletion path.

create_local_bg_thread() holds a reference to the printer via
printer->use while it generates the IPP Everywhere PPD.
cupsdDeleteTemporaryPrinters() respected that reference, but
cupsdDeletePrinter() did not, so an explicit CUPS-Delete-Printer (as
cups-browsed issues when it replaces a discovered queue) freed the
printer mid-thread and corrupted the heap ("corrupted double-linked
list").

Defer deletion while printer->use > 0 by flagging the printer under
printer->lock; cupsdDeleteTemporaryPrinters() then reaps it once the
thread releases its reference.  Extends the use-count protection from
OpenPrinting#1655 to the explicit deletion path.
@andreer

andreer commented Sep 26, 2026

Copy link
Copy Markdown
Author

CI may need this first #1723

@michaelrsweet michaelrsweet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simply this to just decrement use to 0 but not immediately delete it. Will think some, and we should bring this fix to 2.4.x as well.

@michaelrsweet michaelrsweet self-assigned this Sep 26, 2026
@michaelrsweet michaelrsweet added bug Something isn't working priority-high labels Sep 26, 2026
@michaelrsweet michaelrsweet added this to the v2.4.x milestone Sep 26, 2026
michaelrsweet added a commit that referenced this pull request Sep 30, 2026
michaelrsweet added a commit that referenced this pull request Sep 30, 2026
@michaelrsweet

Copy link
Copy Markdown
Member

OK, my fixes:

[master 4bfa89a] Fix potential scheduler printer use-after-free bug (Issue #1722)
[master c1ce727] Minor logic fix for (Issue #1722)

[2.4.x eb06fe2] Fix potential scheduler printer use-after-free bug (Issue #1722)
[2.4.x 2074ceb] Minor logic fix for (Issue #1722)

Basically, I initialize "use" to 1 in cupsdAddPrinter, have cupsdDeletePrinter decrement "use" and only delete when it gets to 0, and update the cupsdDeleteTemporaryPrinters to actually delete any printer whose use count is 0 or a temporary printer whose use count is 1 and it needs to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority-high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants