activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiri Danek <jda...@redhat.com>
Subject Re: [DISCUSS] Coverity Scan for Artemis
Date Sat, 25 Feb 2017 07:47:36 GMT
Coverity sends e-mails with list of new issues it found for every submitted
build. Compared to last week, it now claims 4 new issues. None of them is
actually new. Two appear only now because last time I did not sent the C
code for analysis (in artemis-native) and the other two Java ones two were
there previously (maybe bug in Coverity). See for yourself below.

Regarding impact of the finds:

The mkstemp() thing is supposed to be an issue only with old glibc on Linux
and on AIX and HP-UX. I googled out that there is a good way to set umask()
for the process, because there is JVM running in it too... So probably
ignore.

Variable "iocb" going out of scope leaks the storage it points to. The
first time this is explained in a comment, the second instance of this does
not have a comment. I am not competent to judge, here.

Thread1 sets "state" to a new value. Now the two threads have an
inconsistent view of "state" and updates to fields correlated with "state"
may be lost. Regarding impact, with multithreading I just don't know.

Comparing "threadPool" to null implies that "threadPool" might be null. It
is necessary to look at Coverity website for this as the e-mail contains
only a snippet of the report available on Coverity web. Could it be
possible that  shutdownPool() is referring to wrong object in the log
messages it prints in case of exception? Looking at git history, I think
this is plausible explanation.

On Fri, Feb 24, 2017 at 11:35 PM, <scan-admin@coverity.com> wrote:

Hi,
>
> Please find the latest report on new defect(s) introduced to Apache
> ActiveMQ Artemis found with Coverity Scan.
>
> 4 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
> Scan.
> 7 defect(s), reported by Coverity Scan earlier, were marked fixed in the
> recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 4 of 4 defect(s)
>
>
> ** CID 1411742:  Security best practices violations  (SECURE_TEMP)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 135 in
> JNI_OnLoad()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411742:  Security best practices violations  (SECURE_TEMP)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 135 in
> JNI_OnLoad()
> 129                 fprintf(stderr, "Could not allocate the 1 Mega Buffer
> for initializing files\n");
> 130                 return JNI_ERR;
> 131             }
> 132             memset(oneMegaBuffer, 0, ONE_MEGA);
> 133
> 134             sprintf (dumbPath, "%s/artemisJLHandler_XXXXXX", P_tmpdir);
> >>>     CID 1411742:  Security best practices violations  (SECURE_TEMP)
> >>>     Calling "mkstemp" without securely setting umask first.
> 135             dumbWriteHandler = mkstemp (dumbPath);
> 136
> 137             #ifdef DEBUG
> 138                fprintf (stdout, "Creating temp file %s for dumb
> writes\n", dumbPath);
> 139                fflush(stdout);
> 140             #endif
>
> ** CID 1411741:    (RESOURCE_LEAK)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 368 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 375 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411741:    (RESOURCE_LEAK)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 368 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
> 362            if (iocb[i] == NULL) {
> 363                // it's unlikely this would happen at this point
> 364                // for that reason I'm not cleaning up individual IOCBs
> here
> 365                // we could increase support here with a cleanup of any
> previously allocated iocb
> 366                // But I'm afraid of adding not needed complexity here
> 367                throwOutOfMemoryError(env);
> >>>     CID 1411741:    (RESOURCE_LEAK)
> >>>     Variable "iocb" going out of scope leaks the storage it points to.
> 368                return NULL;
> 369            }
> 370         }
> 371
> 372         struct io_control * theControl = (struct io_control *)
> malloc(sizeof(struct io_control));
> 373         if (theControl == NULL) {
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 375 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
> 369            }
> 370         }
> 371
> 372         struct io_control * theControl = (struct io_control *)
> malloc(sizeof(struct io_control));
> 373         if (theControl == NULL) {
> 374             throwOutOfMemoryError(env);
> >>>     CID 1411741:    (RESOURCE_LEAK)
> >>>     Variable "iocb" going out of scope leaks the storage it points to.
> 375             return NULL;
> 376         }
> 377
> 378         res = pthread_mutex_init(&(theControl->iocbLock), 0);
> 379         if (res) {
> 380             free(theControl);
>
> ** CID 1411740:    (LOCK_EVASION)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411740:    (LOCK_EVASION)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> 996           memoryManager = null;
> 997           backupManager = null;
> 998           storageManager = null;
> 999
> 1000           sessions.clear();
> 1001
> >>>     CID 1411740:    (LOCK_EVASION)
> >>>     Thread1 sets "state" to a new value. Now the two threads have an
> inconsistent view of "state" and updates to fields correlated with "state"
> may be lost.
> 1002           state = SERVER_STATE.STOPPED;
> 1003
> 1004           activationLatch.setCount(1);
> 1005
> 1006           // to display in the log message
> 1007           SimpleString tempNodeID = getNodeID();
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> 996           memoryManager = null;
> 997           backupManager = null;
> 998           storageManager = null;
> 999
> 1000           sessions.clear();
> 1001
> >>>     CID 1411740:    (LOCK_EVASION)
> >>>     Thread1 sets "state" to a new value. Now the two threads have an
> inconsistent view of "state" and updates to fields correlated with "state"
> may be lost.
> 1002           state = SERVER_STATE.STOPPED;
> 1003
> 1004           activationLatch.setCount(1);
> 1005
> 1006           // to display in the log message
> 1007           SimpleString tempNodeID = getNodeID();
>
> ** CID 1411739:  Null pointer dereferences  (FORWARD_NULL)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 968 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411739:  Null pointer dereferences  (FORWARD_NULL)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 968 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> 962           stopComponent(memoryManager);
> 963
> 964           for (SecuritySettingPlugin securitySettingPlugin :
> configuration.getSecuritySettingPlugins()) {
> 965              securitySettingPlugin.stop();
> 966           }
> 967
> >>>     CID 1411739:  Null pointer dereferences  (FORWARD_NULL)
> >>>     Comparing "threadPool" to null implies that "threadPool" might be
> null.
> 968           if (threadPool != null && !threadPoolSupplied) {
> 969              shutdownPool(threadPool);
> 970           }
> 971
> 972           if (ioExecutorPool != null) {
> 973              shutdownPool(ioExecutorPool);
>
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit,
> https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wAHEVPKzmIIOtLlZCpkesdZoOgp56XudeOz1563G73aJe0vuBiY0pxBectz6EdXD0arBJ9IkKlOXWgY2pmVMD4ceyWTtAI2YEWTfDs9EkUt0Cf6E3xISQPQw00kFJBI-2Bfnz1EJpNJbmpbiGiAtWTtMARP9FhjmFQh3KNyIhAPOiCg-3D-3D
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message