573039 - Segfault with CUPS 1.4.4 (Print and Print Preview do not work)

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Richard Fuller]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-GB; rv:1.9.2.3) Gecko/20100611 Namoroka/3.6.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3

Please see http://www.cups.org/str.php?L3606 for detailed information.

Firefox (3.6.3) and Thunderbird (3.0.4) crash when trying to print, or print preview, when using CUPS 1.4.4.

Reproducible: Always

Steps to Reproduce:
1. Open any page
2. File > Print Preview
Actual Results:  
Product exits with segmentation fault.

Expected Results:  
A print preview should be displayed.

The CUPS developer states:

"The problem is that the Mozilla apps are dlopen'ing libcups, which then initializes the SSL library. They then dlclose libcups after the print dialog goes away which leaves the OpenSSL threading stuff pointing at functions that are no longer in the process address space.

One of two things needs to happen - Firefox/Thunderbird need to stop using dlopen/dlclose (or at least dlclose) for libcups, or OpenSSL and GNU TLS need to actually support threading out of the box and not depend on the application or library to provide threading support."

Comment 1

[Boris Zbarsky [:bzbarsky]]

Looks like this has been a problem ever since bug 257381 first landed...

roc, can we avoid the dlopen stuff here?

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No idea. Karl might know.

Comment 3

[Karl Tomlinson (:karlt)]

libcups is not a runtime dependency, so we can't avoid the dlopen by linking directly to the library.
https://wiki.mozilla.org/Linux/Runtime_Requirements

If we can't tell libcups to shut down, then we shouldn't dlclose.
I can't imagine any reason why it must be closed.

I'm assuming from Zach's comments in bug 394413 that libcups is actually still necessary until we add something to replace it?

Comment 4

[Zack Weinberg (:zwol)]

All I know for sure is that when I ripped out the MOZ_ENABLE_POSTSCRIPT code (which is really about direct poking at CUPS) I couldn't make gtk printing work afterward.  That was two years ago, things may have changed.  In principle, Gtk-native apps don't seem to need to poke at CUPS directly AFAIK so we shouldn't need to do it either.

This is not something I have time to look at in the near future.

Comment 5

[Octavian Petre]

As a workaround, until this is fixed, one might use cups 1.4.3

Comment 6

[Martin Stránský [:stransky] (ni? me)]

We have cups-1.4.4 in Fedora 14 so I'm going to work on it.

Comment 7

[Jory A. Pratt [:anarchy]]

(In reply to comment #6)
> We have cups-1.4.4 in Fedora 14 so I'm going to work on it.

Martin If you need anything let me know, I am in process of working on this as well for gentoo.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Hm, I'm not sure why but I'm unable to reproduce in with Fedora 14. 

Package cups-1.4.4-5.fc13.i686, I tested mozilla binary 3.6.7, 3.6.4 from distro and debug build of ff4.b2pre and no problems, printing works fine and print preview too.

Comment 9

[Zack Weinberg (:zwol)]

Is your libcups actually doing anything with libssl?

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Ahh, you're right, we built cups with gnutls for now.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is pretty trivial to work around (simply not call dlclose and leak the library), so I think we should probably block on it, assuming there are actual Linux distros that it affects or will affect.

What distros does this affect?


That said, it also seems pretty bad that using cups once leaves stuff running for the rest of the lifetime of the browser.

Comment 12

[Jory A. Pratt [:anarchy]]

(In reply to comment #11)
> What distros does this affect?

This effects just about all distro's and *bsds. We are all now forcing users to link cups to gnutls to work around the bug.

Comment 13

[Jory A. Pratt [:anarchy]]

(In reply to comment #12)
> (In reply to comment #11)
> > What distros does this affect?
> 
> This effects just about all distro's and *bsds. We are all now forcing users to
> link cups to gnutls to work around the bug.

I rolled a snapshot at changeset b397e6db5067, I am now unable to produce segfault when requesting printing or print preview with ssl built cups. I will try to make sometime to track down what commit has fixed this but seems to be working as expected at the moment.

Comment 14

[Jory A. Pratt [:anarchy]]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > What distros does this affect?
> > 
> > This effects just about all distro's and *bsds. We are all now forcing users to
> > link cups to gnutls to work around the bug.
> 
> I rolled a snapshot at changeset b397e6db5067, I am now unable to produce
> segfault when requesting printing or print preview with ssl built cups. I will
> try to make sometime to track down what commit has fixed this but seems to be
> working as expected at the moment.

It actually looks like openssl-1.0.0a resolves this issue, when cups links with it all seems to work fine. If someone on another distro could confirm some findings would be appreciated.

Comment 16

[Frédéric Buclin]

(In reply to comment #12)
> > What distros does this affect?
> 
> This effects just about all distro's and *bsds.

Yeah, we are getting many reports about this problem:

https://qa.mandriva.com/show_bug.cgi?id=61213
https://qa.mandriva.com/show_bug.cgi?id=61009
https://bugzilla.novell.com/show_bug.cgi?id=617026

This bug affects current stable Linux distros + stable Fx (3.6.11), so more and more people are affected. Any chance to see this problem fixed in 3.6.x too?

Updating the bug summary to make it easier to find.

Comment 17

[Frédéric Buclin]

(In reply to comment #14)
> It actually looks like openssl-1.0.0a resolves this issue, when cups links with
> it all seems to work fine. If someone on another distro could confirm some
> findings would be appreciated.

Jory, Mandriva guys seem to say this doesn't help, see:

https://qa.mandriva.com/show_bug.cgi?id=61009#c25

Comment 18

[Daniel Veditz [:dveditz]]

We're not going to block branch releases on this, but when a reviewed and tested patch lands on trunk we'll take it if it's not introducing too much risk.

Comment 19

[Octavian Petre]

"It actually looks like openssl-1.0.0a resolves this issue."


Gentoo
cups 1.4.4
openssl 1.0.0a
firefox 3.6.10

Comment 20

[Jory A. Pratt [:anarchy]]

(In reply to comment #17)
> (In reply to comment #14)
> > It actually looks like openssl-1.0.0a resolves this issue, when cups links with
> > it all seems to work fine. If someone on another distro could confirm some
> > findings would be appreciated.
> 
> Jory, Mandriva guys seem to say this doesn't help, see:
> 
> https://qa.mandriva.com/show_bug.cgi?id=61009#c25

Are you sure that all apps where relinked to openssl-1.0.1a when testing? Based off what I seen in the post it is not the case.

Comment 21

[Jory A. Pratt [:anarchy]]

(In reply to comment #20)
> (In reply to comment #17)
> > (In reply to comment #14)
> > > It actually looks like openssl-1.0.0a resolves this issue, when cups links with
> > > it all seems to work fine. If someone on another distro could confirm some
> > > findings would be appreciated.
> > 
> > Jory, Mandriva guys seem to say this doesn't help, see:
> > 
> > https://qa.mandriva.com/show_bug.cgi?id=61009#c25
> 
> Are you sure that all apps where relinked to openssl-1.0.1a when testing? Based
> off what I seen in the post it is not the case.

err excuse me I mean 1.0.0a

Comment 22

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sounds like it's not really affecting distros.

Comment 23

[Frédéric Buclin]

(In reply to comment #22)
> Sounds like it's not really affecting distros.

You should re-read comment 16!

Comment 24

[Matthew Gregan [:kinetik]]

Attachment: patch v0

I looked at doing something clever here, but it doesn't seem to be worth the complexity.  The NS_WARNINGs might be unnecessary, but I figured they'd serve as a reminder to clean this up one day.

Comment 25

[Karl Tomlinson (:karlt)]

Comment on attachment 499215 [details] [diff] [review]
patch v0

nsCUPSShims get instantiated and destroyed multiple times.

When not unloading cups, it would be better to use a static variable for the library or static variables for the symbols, than to open the library multiple times.

>             msg.Append(" not found in CUPS library");
>             NS_WARNING(msg.get());
> #endif
>-            PR_UnloadLibrary(mCupsLib);
>+            NS_WARNING("Unable to safely unload CUPS library; leaking reference");

I'm not certain, but I would hope it is safe to unload the library in this case where the library has not been used.

Comment 26

[Matthew Gregan [:kinetik]]

dlopen is refcounted:
       "If the same library is loaded again with dlopen(), the same file handle
       is returned.  The dl library maintains  reference  counts  for  library
       handles,  so  a  dynamic library is not deallocated until dlclose() has
       been called on it as many times as dlopen() has succeeded on  it."

So there doesn't seem to be any benefit to using static variables so that we only open the library once.

Comment 27

[Karl Tomlinson (:karlt)]

I expect dlopen to open the new library should it be reinstalled, which could have the possibility of problems with symbol conflicts when both are installed, but NSPR prevents that because it keeps a mapping from filenames to handles (with reference counts).

So what you are claiming is that we are safe to continue incrementing the reference count either because it is not incremented frequently enough to overflow during the lifetime of the system or because it doesn't get decremented and so is never tested.

That may be the case but it feels ugly to me and is difficult to prove given that these objects are instantiated on the stack.

Is there a problem with using static variable(s)?

Comment 28

[Matthew Gregan [:kinetik]]

Attachment: patch v1

New patch attached that addresses your concerns.

As an aside:
(In reply to comment #27)
> I expect dlopen to open the new library should it be reinstalled, which could

dlopen is keyed on the filename so this does not happen.

Comment 29

[Karl Tomlinson (:karlt)]

Comment on attachment 499233 [details] [diff] [review]
patch v1

>         nsCUPSShim() : mCupsLib(nsnull) { }

>+nsCUPSShim gCupsShim;

With gcc 4.4.4 even -O3 I'm seeing

% nm -C obj/widget/src/gtk2/nsPSPrinters.o | grep global
0000000000000000 t global constructors keyed to nsPSPrinters.cpp

so unless we can know for sure that the static constructor is being optimized away,
I think this approach needs the user-declared mCupsLib constructor removed,
and a commented added to the class indicating that it intended only for static storage (or that default initialization must be performed).

Comment 30

[Karl Tomlinson (:karlt)]

(In reply to comment #29)
>  mCupsLib constructor

I mean nsCUPSShim constructor.

Comment 31

[Karl Tomlinson (:karlt)]

Comment on attachment 499233 [details] [diff] [review]
patch v1

r+ with the static constructor issue addressed.

Comment 32

[Matthew Gregan [:kinetik]]

http://hg.mozilla.org/mozilla-central/rev/6f38fc526313

Comment 33

[Matthew Gregan [:kinetik]]

Comment on attachment 499233 [details] [diff] [review]
patch v1

Sounds like this is needed on the 1.9.2 branch as well.

Comment 34

[LegNeato]

Comment on attachment 499233 [details] [diff] [review]
patch v1

a=LegNeato for 1.9.2.14. We won't block the release on this though.

Comment 35

[Jory A. Pratt [:anarchy]]

Attachment: 1.9.2 patch v1

Patch needed to be updated to apply cleanly to 1.9.2 branch.

Comment 36

[Mounir Lamouri (:mounir)]

Pushed in 1.9.2 branch:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ffa1ef8ab52b

Comment 37

[Jory A. Pratt [:anarchy]]

Attachment: v1 1.9.1 branch

Allow to be applied cleanly

Comment 38

[LegNeato]

Comment on attachment 501308 [details] [diff] [review]
v1 1.9.1 branch

Assuming the status flag request was set to ask approval. Approving for 1.9.1.17.

Comment 39

[Mounir Lamouri (:mounir)]

Pushed in 1.9.1 branch:
https://hg.mozilla.org/releases/mozilla-1.9.1/rev/bf6ce381ea0e

Comment 40

[Al Billings [:abillings - ex-MoCo]]

This doesn't affect all distros by default since I print preview in a downloaded Firefox 3.6.13 on Ubuntu 10.10 without any crashing. Is there a particular mechanism to force the browser to use CUPS?

Comment 41

[Frédéric Buclin]

(In reply to comment #40)
> This doesn't affect all distros by default since I print preview in a
> downloaded Firefox 3.6.13 on Ubuntu 10.10 without any crashing. Is there a
> particular mechanism to force the browser to use CUPS?

Are you sure 1) that CUPS 1.4.4 is installed?, and 2) that Ubuntu didn't already fix the problem themselves? Several distros didn't wait for Mozilla to fix the problem.

Comment 42

[Al Billings [:abillings - ex-MoCo]]

I'm using CUPS 1.4.4. I double-checked. I have no idea of whether Ubuntu fixed it or not. I'm not using their distribution of Firefox. The comments above stated it affected all distros so I used what I had.

Comment 43

[Jory A. Pratt [:anarchy]]

(In reply to comment #42)
> I'm using CUPS 1.4.4. I double-checked. I have no idea of whether Ubuntu fixed
> it or not. I'm not using their distribution of Firefox. The comments above
> stated it affected all distros so I used what I had.

It is not so much the use of cups but rather what lib cups is linked to, Ubuntu as I understand it links to gnutls and not openssl hense you will not see the failure.

Comment 44

[Al Billings [:abillings - ex-MoCo]]

What specific distros will show the failure?

Comment 45

[Jory A. Pratt [:anarchy]]

Hard to say as I do not have time to keep up with all distros, I would check with mandrake or even fedora, easiest way would be to check their bug reports and see if they are closed or still open before you go to installing.

Comment 46

[Frédéric Buclin]

(In reply to comment #44)
> What specific distros will show the failure?

Mandriva and OpenSUSE had the issue for sure, see comment 16. But if you try now, you won't be able to trigger the problem as they fixed the issue meanwhile.