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

Coverity fixes #11892

Closed
wants to merge 4 commits into from
Closed

Coverity fixes #11892

wants to merge 4 commits into from

Conversation

paulidale
Copy link
Contributor

@paulidale paulidale commented May 21, 2020

Fixes for recent Coverity issues:

  • 1463571: Null pointer dereferences (FORWARD_NULL)
  • 1463574: Null pointer dereferences (REVERSE_INULL)
  • 1463576: Error handling issues (CHECKED_RETURN)
  • 1463258: Incorrect expression (EVALUATION_ORDER)

@paulidale paulidale added the branch: master Merge to master branch label May 21, 2020
@kroeckx kroeckx added the approval: done This pull request has the required number of approvals label May 21, 2020
@kroeckx
Copy link
Member

kroeckx commented May 21, 2020

Should that be REVERSE_NULL, without the I?

@paulidale
Copy link
Contributor Author

paulidale commented May 21, 2020

It was a copy paste from the Coverity issue, so I guess it is correct.

I'm not sure what REVERSE_INULL means.

** CID 1463574:  Null pointer dereferences  (REVERSE_INULL)
/providers/implementations/keymgmt/rsa_kmgmt.c: 516 in rsa_gen()


________________________________________________________________________________________________________
*** CID 1463574:  Null pointer dereferences  (REVERSE_INULL)
/providers/implementations/keymgmt/rsa_kmgmt.c: 516 in rsa_gen()
510             break;
511         default:
512             /* Unsupported RSA key sub-type... */
513             return NULL;
514         }
515     
   CID 1463574:  Null pointer dereferences  (REVERSE_INULL)
   Null-checking "gctx" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
516         if (gctx == NULL
517             || (rsa_tmp = rsa_new_with_ctx(gctx->libctx)) == NULL)
518             return NULL;
519     
520         gctx->cb = osslcb;
521         gctx->cbarg = cbarg;

@t8m
Copy link
Member

t8m commented May 21, 2020

It could mean 'is NULL'.

openssl-machine pushed a commit that referenced this pull request May 22, 2020
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #11892)
openssl-machine pushed a commit that referenced this pull request May 22, 2020
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #11892)
openssl-machine pushed a commit that referenced this pull request May 22, 2020
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #11892)
openssl-machine pushed a commit that referenced this pull request May 22, 2020
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from #11892)
@paulidale
Copy link
Contributor Author

Merged to master, thanks for the review.

@paulidale paulidale closed this May 22, 2020
@paulidale paulidale deleted the coverity branch May 22, 2020 07:26
@paulidale paulidale added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants