Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

printer-driver.c: Fix crash when adding generic PS printer #322

Closed
wants to merge 1 commit into from

Conversation

zdohnal
Copy link
Contributor

@zdohnal zdohnal commented Jan 19, 2024

If legacy-printer-app is run for the first time and generic PS driver was chosen when installing the printer, the application crashes during adding the printer.

It is because the printer structure does not have device_id set, and the library tries to get data from already freed printer->driver_attrs. Additionally, if the device id exists in the printer, it rewrites incorrect structure (we should set device id attr in attrs, which we return).

The fix adds possibility to extract device id from printer->attrs and if device id is missing in printer->attrs and document-format-supported as well, try document-format-default and if this attribute is missing, return NULL.

If legacy-printer-app is run for the first time and generic PS driver
was chosen when installing the printer, the application crashes during
adding the printer.

It is because the printer structure does not have device_id set, and the
library tries to get data from already freed `printer->driver_attrs`.
Additionally, if the device id exists in the printer, it rewrites
incorrect structure (we should set device id attr in `attrs`, which we
return).

The fix adds possibility to extract device id from `printer->attrs` and if
device id is missing in `printer->attrs` and `document-format-supported`
as well, try `document-format-default` and if this attribute is missing,
return NULL.
@zdohnal
Copy link
Contributor Author

zdohnal commented Jan 19, 2024

The key for reproducing the issue is that it must be the first run of legacy-printer-app after its installation and start. I was not able to reproduce in other runs.

@michaelrsweet
Copy link
Owner

Hard to diagnose this without a backtrace...

Copy link
Owner

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

Need more information before we can decide this is an appropriate set of fixes.

@zdohnal
Copy link
Contributor Author

zdohnal commented Jan 19, 2024

Full backtrace of the crash - the program crashes in libcups, because it tries to access invalid memory, which was freed in pappl, however it is accessed later during generating driver attributes. It is the thread 1, in ippFindNextAttribute() function.

Copy link
Owner

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

OK, I've had a chance to go through this and yes there are a couple bugs here but the fixes you've proposed aren't quite right. I'll be pushing the correct fixes shortly to both the master and v1.4.x branches. But thank you for catching this!

ippAddString(printer->attrs, IPP_TAG_PRINTER, IPP_TAG_TEXT, "printer-device-id", NULL, printer->device_id);
ippAddString(attrs, IPP_TAG_PRINTER, IPP_TAG_TEXT, "printer-device-id", NULL, printer->device_id);
}
else if ((attr = ippFindAttribute(printer->attrs, "printer-device-id", IPP_TAG_PRINTER)) != NULL)
Copy link
Owner

Choose a reason for hiding this comment

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

After looking through this, we don't want to look for printer-device-id in printer->attrs since it shouldn't be there and you'd end up with duplicate printer-device-id attrs.

@@ -986,7 +990,10 @@ make_attrs(
else
mdl = mfg; // No separator, so assume the make and model are the same

formats = ippFindAttribute(printer->driver_attrs, "document-format-supported", IPP_TAG_MIMETYPE);
if ((formats = ippFindAttribute(printer->attrs, "document-format-supported", IPP_TAG_MIMETYPE)) == NULL)
Copy link
Owner

Choose a reason for hiding this comment

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

This should just be looking in attrs since printer->attrs won't have document-format-supported (driver-specific) and we just want document-format-supported since document-format-default is just the one default and usually application/octet-stream.

michaelrsweet added a commit that referenced this pull request Jan 22, 2024
@michaelrsweet
Copy link
Owner

[master 47cfc65] Fix printer-device-id (Issue #322)

[v1.4.x 8820820] Bump version/copyright.

@michaelrsweet michaelrsweet self-assigned this Jan 22, 2024
@michaelrsweet michaelrsweet added bug Something isn't working priority-medium labels Jan 22, 2024
@michaelrsweet michaelrsweet added this to the Stable milestone Jan 22, 2024
michaelrsweet added a commit that referenced this pull request Jan 23, 2024
@zdohnal
Copy link
Contributor Author

zdohnal commented Jan 23, 2024

Thank you for the fix, Mike! It works in my testing env.

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-medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants