Bug 695011

The PEM module's initialization function pem_Initialize we has
....
    open_log(); 
    plog("pem_Initialize\n");
....
Looing is implemented in util.c with
FILE *plogfile;
void open_log()
{
#ifdef DEBUG
    plogfile = fopen("/tmp/pkcs11.log", "a");
#endif

    return;
}
The fopen tries to open the log file to write to it. If the log file was initilially created by an application running as root fopen will fail and plogfile will be set to NULL. Later plog calls
    vfprintf(plogfile, fmt, ap);
with a NULL and a crash happens.

Using the logging utilities from NSPR will prevent this problems. It is also what's required of modules intended to be part of NSS as this one is.

Created attachment 490990 [details]
Reimplement PEM logging in terms of NSPR's own

Comment on attachment 490990 [details]
Reimplement PEM logging in terms of NSPR's own

>@@ -267,34 +271,35 @@ ReadDERFromFile(SECItem *** derlist, char *filename, PRBool ascii,
>     return -1;
> }
> 
>-FILE *plogfile;
>+#define PEM_DEFAULT_LOG_FILE "/tmp/pkcs11.log"
>+
>+static const char *pemLogModuleName = "PEM";
>+
>+PRLogModuleInfo* pemLogModule;

pemLogModule could be static.  Both definitions should also be wrapped by "#ifdef DEBUG" so that they do not provoke picky compilers/analyzers when building without DEBUG.

> void open_log()
> {
> #ifdef DEBUG
>-    plogfile = fopen("/tmp/pkcs11.log", "a");
>-#endif
>+	const char *nsprLogFile = PR_GetEnv("NSPR_LOG_FILE");
> 
>-    return;
>-}
>+	pemLogModule = PR_NewLogModule(pemLogModuleName);

The above should be indented by 4 spaces, not a tab.

>-void close_log()
>-{
>-#ifdef DEBUG
>-    fclose(plogfile);
>+	(void) PR_SetLogFile(nsprLogFile ? nsprLogFile : PEM_DEFAULT_LOG_FILE);
>+	/* If false, the log file will remain what it was before */
> #endif
>-    return;
> }

The above should be indented by 4 spaces, not a tab.

>+#define LOGGING_BUFFER_SIZE 400
>+
> void plog(const char *fmt, ...)
> {
> #ifdef DEBUG
>     va_list ap;
>+    char buf[LOGGING_BUFFER_SIZE];
> 
>     va_start(ap, fmt);
>-    vfprintf(plogfile, fmt, ap);
>+    PR_vsnprintf(buf, sizeof(buf), fmt, ap);
>     va_end(ap);
>-
>-    fflush(plogfile);
>+    PR_LOG(pemLogModule, PR_LOG_DEBUG, ("%s", buf));
> #endif
> }

This looks like a reasonable mid-term solution.  The more ambitious solution would be to replace all the calls of plog() by PR_LOG() in order to avoid computation of the output when not printed anyway.  This would probably allow us to drop the #ifdefs completely and compile the debugging support by default.  But for now, I am fine with this solution.

(In reply to comment #3)
> pemLogModule could be static.  Both definitions should also be wrapped by
> "#ifdef DEBUG" so that they do not provoke picky compilers/analyzers when
> building without DEBUG.
Yes
> The above should be indented by 4 spaces, not a tab.
> The above should be indented by 4 spaces, not a tab.
Sure, I'll fix it.

> This looks like a reasonable mid-term solution.  The more ambitious solution
> would be to replace all the calls of plog() by PR_LOG() in order to avoid
> computation of the output when not printed anyway.  This would probably allow
> us to drop the #ifdefs completely and compile the debugging support by default.
>  But for now, I am fine with this solution.
Yes, this short-term (and not to disruptive solution) is prudent given that f-15 is going beta. The more ambitious solution is in progress and is best saved for the NSS upstream submission.

Created attachment 491545 [details]
Patch V2: with changes from Kamil's review

nss-3.12.9-15.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/nss-3.12.9-15.fc15

nss-3.12.9-10.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/nss-3.12.9-10.fc14

Package nss-3.12.9-15.fc15:
* should fix your issue,
* was pushed to the Fedora 15 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing nss-3.12.9-15.fc15'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/nss-3.12.9-15.fc15
then log in and leave karma (feedback).

nss-3.12.9-15.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

nss-3.12.9-10.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.