[Gluster-devel] Fwd: New Defects reported by Coverity Scan for GlusterFS

Atin Mukherjee amukherj at redhat.com
Mon Dec 1 07:02:48 UTC 2014



On 12/01/2014 12:31 PM, Vijay Bellur wrote:
> Hi All,
> 
> Shall we set a goal for ourselves to be Coverity Scan clean by GlusterFS
> 3.7?
> 
> I think fixing problems reported in the incremental reports here would
> be a good way of keeping the number of static analysis defects in
> control. It would be great if developers who checked in code recently to
> the files mentioned in these reports pay attention to the results.
+1
> 
> Thanks,
> Vijay
> 
> On 11/28/2014 12:37 PM, Lalatendu Mohanty wrote:
>>
>> Guideline for fixing Coverity issues :
>> http://www.gluster.org/community/documentation/index.php/Fixing_Issues_Reported_By_Tools_For_Static_Code_Analysis#Coverity
>>
>>
>> Thanks,
>> Lala
>>
>> -------- Forwarded Message --------
>> Subject:     New Defects reported by Coverity Scan for GlusterFS
>> Date:     Thu, 27 Nov 2014 12:31:06 -0800
>> From:     scan-admin at coverity.com
>> To:     lala at redhat.com
>>
>>
>>
>> Hi,
>>
>> Please find the latest report on new defect(s) introduced to GlusterFS
>> found with Coverity Scan.
>>
>> 13 new defect(s) introduced to GlusterFS found with Coverity Scan.
>> 97 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 13 of 13 defect(s)
>>
>>
>> ** CID 1256178:  Logically dead code  (DEADCODE)
>> /api/src/glfs.c: 153 in glusterfs_ctx_defaults_init()
>>
>> ** CID 1256180:  Logically dead code  (DEADCODE)
>> /api/src/glfs.c: 161 in glusterfs_ctx_defaults_init()
>>
>> ** CID 1256176:  Logically dead code  (DEADCODE)
>> /glusterfsd/src/glusterfsd.c: 1426 in glusterfs_ctx_defaults_init()
>>
>> ** CID 1256179:  Dereference after null check  (FORWARD_NULL)
>> /xlators/nfs/server/src/mount3.c: 1082 in mnt3_readlink_cbk()
>>
>> ** CID 1256177:  Explicit null dereferenced  (FORWARD_NULL)
>> /api/src/glfs-fops.c: 702 in pub_glfs_preadv_async()
>>
>> ** CID 1256175:  Array compared against 0  (NO_EFFECT)
>> /xlators/mgmt/glusterd/src/glusterd-snapshot.c: 2433 in
>> glusterd_lvm_snapshot_remove()
>> /xlators/mgmt/glusterd/src/glusterd-snapshot.c: 2433 in
>> glusterd_lvm_snapshot_remove()
>>
>> ** CID 1256173:  Thread deadlock  (ORDER_REVERSAL)
>> /xlators/cluster/ec/src/ec-common.c: 1335 in ec_unlock_timer_add()
>>
>> ** CID 1256174:  Copy into fixed size buffer  (STRING_OVERFLOW)
>> /xlators/mgmt/glusterd/src/glusterd.c: 287 in glusterd_dump_peer()
>>
>> ** CID 1256172:  Copy into fixed size buffer  (STRING_OVERFLOW)
>> /xlators/mgmt/glusterd/src/glusterd.c: 330 in
>> glusterd_dump_peer_rpcstat()
>>
>> ** CID 1256171:  Copy into fixed size buffer  (STRING_OVERFLOW)
>> /xlators/mgmt/glusterd/src/glusterd-handshake.c: 279 in
>> build_volfile_path()
>>
>> ** CID 1238183:  Missing break in switch  (MISSING_BREAK)
>> /xlators/mgmt/glusterd/src/glusterd-rebalance.c: 577 in
>> glusterd_op_stage_rebalance()
>>
>> ** CID 1228602:  Use of untrusted scalar value  (TAINTED_SCALAR)
>> /xlators/mount/fuse/src/fuse-bridge.c: 4843 in fuse_thread_proc()
>>
>> ** CID 1228603:  Use of untrusted scalar value  (TAINTED_SCALAR)
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>>
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256178:  Logically dead code  (DEADCODE)
>> /api/src/glfs.c: 153 in glusterfs_ctx_defaults_init()
>> 147
>> 148         pthread_mutex_init (&(ctx->lock), NULL);
>> 149
>> 150         ret = 0;
>> 151     err:
>> 152         if (ret && pool) {
>>>>>     CID 1256178:  Logically dead code  (DEADCODE)
>>>>>     Execution cannot reach this statement "if (pool->frame_mem_pool)
>>   ...".
>> 153             if (pool->frame_mem_pool)
>> 154                 mem_pool_destroy (pool->frame_mem_pool);
>> 155             if (pool->stack_mem_pool)
>> 156                 mem_pool_destroy (pool->stack_mem_pool);
>> 157             GF_FREE (pool);
>> 158         }
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256180:  Logically dead code  (DEADCODE)
>> /api/src/glfs.c: 161 in glusterfs_ctx_defaults_init()
>> 155             if (pool->stack_mem_pool)
>> 156                 mem_pool_destroy (pool->stack_mem_pool);
>> 157             GF_FREE (pool);
>> 158         }
>> 159
>> 160         if (ret && ctx) {
>>>>>     CID 1256180:  Logically dead code  (DEADCODE)
>>>>>     Execution cannot reach this statement "if (ctx->stub_mem_pool)
>>    m...".
>> 161             if (ctx->stub_mem_pool)
>> 162                 mem_pool_destroy (ctx->stub_mem_pool);
>> 163             if (ctx->dict_pool)
>> 164                 mem_pool_destroy (ctx->dict_pool);
>> 165             if (ctx->dict_data_pool)
>> 166                 mem_pool_destroy (ctx->dict_data_pool);
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256176:  Logically dead code  (DEADCODE)
>> /glusterfsd/src/glusterfsd.c: 1426 in glusterfs_ctx_defaults_init()
>> 1420             lim.rlim_max = RLIM_INFINITY;
>> 1421             setrlimit (RLIMIT_CORE, &lim);
>> 1422
>> 1423             ret = 0;
>> 1424     out:
>> 1425
>>>>>     CID 1256176:  Logically dead code  (DEADCODE)
>>>>>     Execution cannot reach this expression "ctx" inside statement
>>>>> "if (ret && ctx) {
>>    if (ctx...".
>> 1426             if (ret && ctx) {
>> 1427                     if (ctx->pool) {
>> 1428                             mem_pool_destroy
>> (ctx->pool->frame_mem_pool);
>> 1429                             mem_pool_destroy
>> (ctx->pool->stack_mem_pool);
>> 1430                     }
>> 1431                     GF_FREE (ctx->pool);
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256179:  Dereference after null check  (FORWARD_NULL)
>> /xlators/nfs/server/src/mount3.c: 1082 in mnt3_readlink_cbk()
>> 1076             GF_FREE (relative_path);
>> 1077
>> 1078             return ret;
>> 1079
>> 1080     mnterr:
>> 1081             mntstat = mnt3svc_errno_to_mnterr (-ret);
>>>>>     CID 1256179:  Dereference after null check  (FORWARD_NULL)
>>>>>     Dereferencing null pointer "mres".
>> 1082             mnt3svc_mnt_error_reply (mres->req, mntstat);
>> 1083             if (absolute_path)
>> 1084                     GF_FREE (absolute_path);
>> 1085             if (parent_path)
>> 1086                     GF_FREE (parent_path);
>> 1087             if (relative_path)
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256177:  Explicit null dereferenced  (FORWARD_NULL)
>> /api/src/glfs-fops.c: 702 in pub_glfs_preadv_async()
>> 696                            void *data)
>> 697     {
>> 698         struct glfs_io *gio = NULL;
>> 699         int             ret = 0;
>> 700         call_frame_t   *frame = NULL;
>> 701         xlator_t       *subvol = NULL;
>>>>>     CID 1256177:  Explicit null dereferenced  (FORWARD_NULL)
>>>>>     Assigning: "fs" = "NULL".
>> 702         glfs_t         *fs = NULL;
>> 703         fd_t           *fd = NULL;
>> 704
>> 705         __glfs_entry_fd (glfd);
>> 706
>> 707         subvol = priv_glfs_active_subvol (glfd->fs);
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256175:  Array compared against 0  (NO_EFFECT)
>> /xlators/mgmt/glusterd/src/glusterd-snapshot.c: 2433 in
>> glusterd_lvm_snapshot_remove()
>> 2427                             }
>> 2428
>> 2429                             continue;
>> 2430                     }
>> 2431
>> 2432                     /* Check if the brick has a LV associated
>> with it */
>>>>>     CID 1256175:  Array compared against 0  (NO_EFFECT)
>>>>>     Comparing an array to null is not useful:
>>>>> "!brickinfo->device_path".
>> 2433                     if (!brickinfo->device_path) {
>> 2434                             gf_log (this->name, GF_LOG_DEBUG,
>> 2435                                     "Brick (%s:%s) does not have
>> a LV "
>> 2436                                     "associated with it. Removing
>> the brick path",
>> 2437                                     brickinfo->hostname,
>> brickinfo->path);
>> 2438                             goto remove_brick_path;
>> /xlators/mgmt/glusterd/src/glusterd-snapshot.c: 2433 in
>> glusterd_lvm_snapshot_remove()
>> 2427                             }
>> 2428
>> 2429                             continue;
>> 2430                     }
>> 2431
>> 2432                     /* Check if the brick has a LV associated
>> with it */
>>>>>     CID 1256175:  Array compared against 0  (NO_EFFECT)
>>>>>     Comparing an array to null is not useful:
>>>>> "brickinfo->device_path".
>> 2433                     if (!brickinfo->device_path) {
>> 2434                             gf_log (this->name, GF_LOG_DEBUG,
>> 2435                                     "Brick (%s:%s) does not have
>> a LV "
>> 2436                                     "associated with it. Removing
>> the brick path",
>> 2437                                     brickinfo->hostname,
>> brickinfo->path);
>> 2438                             goto remove_brick_path;
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256173:  Thread deadlock  (ORDER_REVERSAL)
>> /xlators/cluster/ec/src/ec-common.c: 1335 in ec_unlock_timer_add()
>> 1329         } else {
>> 1330             ec_trace("UNLOCK_DELAY", fop, "lock=%p", lock);
>> 1331
>> 1332             delay.tv_sec = 1;
>> 1333             delay.tv_nsec = 0;
>> 1334
>>>>>     CID 1256173:  Thread deadlock  (ORDER_REVERSAL)
>>>>>     Calling "pthread_spin_lock(pthread_spinlock_t *)" acquires lock
>>>>> "_ec_fop_data.lock" while holding lock "_inode.lock" (count: 1 / 3).
>> 1335             LOCK(&fop->lock);
>> 1336
>> 1337             fop->jobs++;
>> 1338             fop->refs++;
>> 1339
>> 1340             UNLOCK(&fop->lock);
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256174:  Copy into fixed size buffer  (STRING_OVERFLOW)
>> /xlators/mgmt/glusterd/src/glusterd.c: 287 in glusterd_dump_peer()
>> 281     glusterd_dump_peer (glusterd_peerinfo_t *peerinfo, char
>> *input_key, int index,
>> 282                         gf_boolean_t xpeers)
>> 283     {
>> 284             char   subkey[50]               = {0,};
>> 285             char   key[GF_DUMP_MAX_BUF_LEN] = {0,};
>> 286
>>>>>     CID 1256174:  Copy into fixed size buffer  (STRING_OVERFLOW)
>>>>>     Note: This defect has an elevated risk because the source
>>>>> argument is a parameter of the current function.
>> 287             strcpy (key, input_key);
>> 288
>> 289             snprintf (subkey, sizeof (subkey), "%s%d", key, index);
>> 290
>> 291             gf_proc_dump_build_key (key, subkey, "uuid");
>> 292             gf_proc_dump_write (key, "%s",
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256172:  Copy into fixed size buffer  (STRING_OVERFLOW)
>> /xlators/mgmt/glusterd/src/glusterd.c: 330 in
>> glusterd_dump_peer_rpcstat()
>> 324             int                   
>> ret                                 = -1;
>> 325             rpc_clnt_t           
>> *rpc                                 = NULL;
>> 326             char                  
>> rpcsvc_peername[RPCSVC_PEER_STRLEN] = {0,};
>> 327             char                  
>> subkey[50]                          = {0,};
>> 328             char                  
>> key[GF_DUMP_MAX_BUF_LEN]            = {0,};
>> 329
>>>>>     CID 1256172:  Copy into fixed size buffer  (STRING_OVERFLOW)
>>>>>     Note: This defect has an elevated risk because the source
>>>>> argument is a parameter of the current function.
>> 330             strcpy (key, input_key);
>> 331
>> 332             /* Dump the rpc connection statistics */
>> 333             rpc = peerinfo->rpc;
>> 334             if (rpc) {
>> 335                     conn = &rpc->conn;
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1256171:  Copy into fixed size buffer  (STRING_OVERFLOW)
>> /xlators/mgmt/glusterd/src/glusterd-handshake.c: 279 in
>> build_volfile_path()
>> 273             if (ret == -1)
>> 274                     goto out;
>> 275
>> 276             ret = stat (path, &stbuf);
>> 277
>> 278             if ((ret == -1) && (errno == ENOENT)) {
>>>>>     CID 1256171:  Copy into fixed size buffer  (STRING_OVERFLOW)
>>>>>     You might overrun the 4096 byte fixed-size string "dup_volid"
>>>>> by copying "volid_ptr" without checking the length.
>> 279                     strcpy (dup_volid, volid_ptr);
>> 280                     if (!strchr (dup_volid, '.')) {
>> 281                             switch (volinfo->transport_type) {
>> 282                             case GF_TRANSPORT_TCP:
>> 283                                     strcat (dup_volid, ".tcp");
>> 284                                     break;
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1238183:  Missing break in switch  (MISSING_BREAK)
>> /xlators/mgmt/glusterd/src/glusterd-rebalance.c: 577 in
>> glusterd_op_stage_rebalance()
>> 571                                                "disconnect those
>> clients before "
>> 572                                                "attempting this
>> command again.",
>> 573                                                volname);
>> 574                             goto out;
>> 575                     }
>> 576
>>>>>     CID 1238183:  Missing break in switch  (MISSING_BREAK)
>>>>>     The above case falls through to this one.
>> 577             case GF_DEFRAG_CMD_START_FORCE:
>> 578                     if (is_origin_glusterd (dict)) {
>> 579                             op_ctx = glusterd_op_get_ctx ();
>> 580                             if (!op_ctx) {
>> 581                                     ret = -1;
>> 582                                     gf_log (this->name, GF_LOG_ERROR,
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1228602:  Use of untrusted scalar value  (TAINTED_SCALAR)
>> /xlators/mount/fuse/src/fuse-bridge.c: 4843 in fuse_thread_proc()
>> 4837                                     "short read on /dev/fuse");
>> 4838                             fuse_log_eh (this, "glusterfs-fuse:
>> short read on "
>> 4839                                          "/dev/fuse");
>> 4840                             break;
>> 4841                     }
>> 4842
>>>>>     CID 1228602:  Use of untrusted scalar value  (TAINTED_SCALAR)
>>>>>     Assigning: "finh" = "(fuse_in_header_t *)iov_in[0].iov_base".
>>>>> Both are now tainted.
>> 4843                     finh = (fuse_in_header_t *)iov_in[0].iov_base;
>> 4844
>> 4845                     if (res != finh->len
>> 4846     #ifdef GF_DARWIN_HOST_OS
>> 4847                         /* work around fuse4bsd/MacFUSE msg size
>> miscalculation bug,
>> 4848                          * that is, payload size is not taken
>> into account for
>>
>> ________________________________________________________________________________________________________
>>
>> *** CID 1228603:  Use of untrusted scalar value  (TAINTED_SCALAR)
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> 2125                                     lines = NULL;
>> 2126                                     goto out;
>> 2127                             }
>> 2128                             lines = p;
>> 2129                     }
>> 2130
>>>>>     CID 1228603:  Use of untrusted scalar value  (TAINTED_SCALAR)
>>>>>     Assigning: "lines[counter]" = "gf_strdup(char const *)", which
>>>>> taints "lines[counter]".
>> 2131                     lines[counter] = gf_strdup (buffer);
>> 2132             }
>> 2133
>> 2134             lines[counter] = NULL;
>> 2135             /* Reduce allocation to minimal size.  */
>> 2136             p = GF_REALLOC (lines, (counter + 1) * sizeof (char *));
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> 2125                                     lines = NULL;
>> 2126                                     goto out;
>> 2127                             }
>> 2128                             lines = p;
>> 2129                     }
>> 2130
>>>>>     CID 1228603:  Use of untrusted scalar value  (TAINTED_SCALAR)
>>>>>     Assigning: "lines[counter]" = "gf_strdup(char const *)", which
>>>>> taints "lines[counter]".
>> 2131                     lines[counter] = gf_strdup (buffer);
>> 2132             }
>> 2133
>> 2134             lines[counter] = NULL;
>> 2135             /* Reduce allocation to minimal size.  */
>> 2136             p = GF_REALLOC (lines, (counter + 1) * sizeof (char *));
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> 2125                                     lines = NULL;
>> 2126                                     goto out;
>> 2127                             }
>> 2128                             lines = p;
>> 2129                     }
>> 2130
>>>>>     CID 1228603:  Use of untrusted scalar value  (TAINTED_SCALAR)
>>>>>     Assigning: "lines[counter]" = "gf_strdup(char const *)", which
>>>>> taints "lines[counter]".
>> 2131                     lines[counter] = gf_strdup (buffer);
>> 2132             }
>> 2133
>> 2134             lines[counter] = NULL;
>> 2135             /* Reduce allocation to minimal size.  */
>> 2136             p = GF_REALLOC (lines, (counter + 1) * sizeof (char *));
>> /xlators/mgmt/glusterd/src/glusterd-utils.c: 2131 in
>> glusterd_readin_file()
>> 2125                                     lines = NULL;
>> 2126                                     goto out;
>> 2127                             }
>> 2128                             lines = p;
>> 2129                     }
>> 2130
>>>>>     CID 1228603:  Use of untrusted scalar value  (TAINTED_SCALAR)
>>>>>     Assigning: "lines[counter]" = "gf_strdup(char const *)", which
>>>>> taints "lines[counter]".
>> 2131                     lines[counter] = gf_strdup (buffer);
>> 2132             }
>> 2133
>> 2134             lines[counter] = NULL;
>> 2135             /* Reduce allocation to minimal size.  */
>> 2136             p = GF_REALLOC (lines, (counter + 1) * sizeof (char *));
>>
>>
>> ________________________________________________________________________________________________________
>>
>> To view the defects in Coverity Scan
>> visit,http://scan.coverity.com/projects/987?tab=overview
>>
>> To unsubscribe from the email notification for new
>> defects,http://scan5.coverity.com/cgi-bin/unsubscribe.py
>>
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>> Gluster-devel mailing list
>> Gluster-devel at gluster.org
>> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
>>
> 
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel


More information about the Gluster-devel mailing list