321600 - SECMOD_LoadPKCS11Module calls to PR_GetLibraryName must use PR_FreeLibraryName

Status: NEW

(NSS :: Libraries, defect, P5)

Description

[timeless]

it's not a good idea to leave bad code around, someone might thing it's a good sample and copy it :(.

Comment 1

[timeless]

Attachment: make alternate codepath correct

Comment 2

[Wan-Teh Chang]

Comment on attachment 206912 [details] [diff] [review]
make alternate codepath correct

I found that the return value of PR_GetLibraryPath
also needs to be freed, but there is no
PR_FreeLibraryPath function, so we need to use
PR_Free to free it.

We should just remove the #ifdef notdef code.
If we do that, we should just pass mod->dllName
to PR_LoadLibrary and get rid of the full_name
variable and the PORT_Strdup call.

Comment 3

[timeless]

yeah PR_GetLibraryPath worried me, but i couldn't figure out what to do with it :).

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Alexei, please check this in on the trunk for 3.12

Comment 5

[Alexei Volkov]

/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11load.c,v  <--  pk11load.c
new revision: 1.20; previous revision: 1.19

the code still have know mem leak(see comment 2). Will need to make another patch.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

(In reply to comment #2)
> (From update of attachment 206912 [details] [diff] [review])
> I found that the return value of PR_GetLibraryPath also needs to be freed, 
> but there is no PR_FreeLibraryPath function, so we need to use PR_Free to 
> free it.

Wan-Teh, 
There are no calls to PR_GetLibraryPath in NSS, so how is this comment
relevant to this bug?

Your comment 2 says "We should just remove the #ifdef notdef code."
That's exactly the code that timeless's patch added.
I think you meant that we should keep the ifdef notdef code and get rid
of the other path.  

But PR_FreeLibraryName is just a synonym for PR_Free, so this whole bug
seems pointless.  Let's work on fixing real bugs that cause real problems.
I vote for WONTFIX.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Unsetting target milestone in unresolved bugs whose targets have passed.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.