Confusion over request being NULL in v3.0.x cbtls_new_session #1936

Closed
spbnick opened this Issue Mar 10, 2017 · 0 comments

Comments

Projects
None yet
1 participant
Collaborator

spbnick commented Mar 10, 2017

Issue type

  • Defect - Crash or memory corruption.
  • Defect - Non compliance with a standards document, or incorrect API usage.
  • Defect - Unexpected behaviour (obvious or verified by project member).
  • Feature request.

Defect/Feature description

Coverity complains about request being possibly NULL and assumed not being
NULL at the same time in cbtls_new_session in v3.0.x branch. Perhaps a check
should be put at the start of the function and the if (request) condition
removed further into the function, or perhaps all the invocations of *DEBUG
should be conditional. I'm not sure what to do myself.

Here are the warnings:

 14. freeradius-server-3.0.13/src/main/tls.c:1395: var_compare_op: Comparing "request" to null implies that "request" might be null.
23. freeradius-server-3.0.13/src/main/tls.c:1420: var_deref_model: Passing null pointer "request" to "radlog_request", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
#  1418|   		}
#  1419|   		close(fd);
#  1420|-> 		RWDEBUG("Wrote session %s to %s (%d bytes)", buffer, filename, blob_len);
#  1421|   	}
#  1422|   

 14. freeradius-server-3.0.13/src/main/tls.c:1395: var_compare_op: Comparing "request" to null implies that "request" might be null.
18. freeradius-server-3.0.13/src/main/tls.c:1412: var_deref_model: Passing null pointer "request" to "radlog_request", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
#  1410|   			rv = write(fd, p, todo);
#  1411|   			if (rv < 1) {
#  1412|-> 				RWDEBUG("Failed writing session: %s", fr_syserror(errno));
#  1413|   				close(fd);
#  1414|   				goto error;

1. freeradius-server-3.0.13/src/main/tls.c:1357: deref_ptr_in_call: Dereferencing pointer "request". (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
2. freeradius-server-3.0.13/src/main/tls.c:1395: check_after_deref: Null-checking "request" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
#  1393|   		 *	Set the filename to be temporarily write-only.
#  1394|   		 */
#  1395|-> 		if (request) {
#  1396|   			VALUE_PAIR *vp;
#  1397|   

@alandekok alandekok added a commit that referenced this issue Mar 10, 2017

@alandekok alandekok Allo session resumption for RadSec connectins. Closes #1936 43efa43

spbnick closed this Mar 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment