Fwd: New Defects reported by Coverity Scan for git

Fwd: New Defects reported by Coverity Scan for git

[Duy Nguyen]

Jeff, I suppose you are the admin of git on scan.coverity, or knows
him/her, perhaps we can add a model for xmalloc to suppress these
"null pointer deferences" reports? We are sure xmalloc() never returns
NULL. Qemu did it [1] and it looks simple.. I think something like
this would do

void *xmalloc(size_t size)
{
   void *mem = malloc(size);
   if (!mem) __coverity_panic__();
   return mem;
}

[1] http://git.qemu.org/?p=qemu.git;a=blob;f=scripts/coverity-model.c;h=4c99a85cfc292caa9edd9d041e2683ee53490a8d;hb=e40cdb0e6efb795e4d19368987d53e3e4ae19cf7#l104


---------- Forwarded message ----------
From:  <scan-admin@coverity.com>
Date: Fri, Jul 31, 2015 at 5:54 PM
Subject: New Defects reported by Coverity Scan for git
To: pclouds@gmail.com

_______________________________________________________________________________________________________
*** CID 1313836:  Null pointer dereferences  (FORWARD_NULL)
/rerere.c: 150 in find_rerere_dir()
144                     return NULL; /* BUG */
145             pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr,
rerere_dir_sha1);
146             if (pos < 0) {
147                     rr_dir = xmalloc(sizeof(*rr_dir));
148                     hashcpy(rr_dir->sha1, sha1);
149                     rr_dir->status_nr = rr_dir->status_alloc = 0;
>>>     CID 1313836:  Null pointer dereferences  (FORWARD_NULL)
>>>     Assigning: "rr_dir->status" = "NULL".
150                     rr_dir->status = NULL;
151                     pos = -1 - pos;
152
153                     /* Make sure the array is big enough ... */
154                     ALLOC_GROW(rerere_dir, rerere_dir_nr + 1,
rerere_dir_alloc);
155                     /* ... and add it in. */

** CID 1313835:  Null pointer dereferences  (FORWARD_NULL)
/builtin/fetch.c: 795 in prune_refs()
-- 
Duy

Re: New Defects reported by Coverity Scan for git

[Stefan Beller]

On Fri, Jul 31, 2015 at 4:24 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> Jeff, I suppose you are the admin of git on scan.coverity, or knows
> him/her, perhaps we can add a model for xmalloc to suppress these
> "null pointer deferences" reports? We are sure xmalloc() never returns
> NULL. Qemu did it [1] and it looks simple.. I think something like
> this would do
>
> void *xmalloc(size_t size)
> {
>    void *mem = malloc(size);
>    if (!mem) __coverity_panic__();
>    return mem;
> }
>
> [1] http://git.qemu.org/?p=qemu.git;a=blob;f=scripts/coverity-model.c;h=4c99a85cfc292caa9edd9d041e2683ee53490a8d;hb=e40cdb0e6efb795e4d19368987d53e3e4ae19cf7#l104
>

Taking just that excerpt doesn't work. Upload fails with
"modeling_file.c", line 12: error #20:
          identifier "malloc" is undefined
  void *mem = malloc(size);

I'll look into your reference[1] a bit more and try to follow it as a guidance.


>
> ---------- Forwarded message ----------
> From:  <scan-admin@coverity.com>
> Date: Fri, Jul 31, 2015 at 5:54 PM
> Subject: New Defects reported by Coverity Scan for git
> To: pclouds@gmail.com
>
> _______________________________________________________________________________________________________
> *** CID 1313836:  Null pointer dereferences  (FORWARD_NULL)
> /rerere.c: 150 in find_rerere_dir()
> 144                     return NULL; /* BUG */
> 145             pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr,
> rerere_dir_sha1);
> 146             if (pos < 0) {
> 147                     rr_dir = xmalloc(sizeof(*rr_dir));
> 148                     hashcpy(rr_dir->sha1, sha1);
> 149                     rr_dir->status_nr = rr_dir->status_alloc = 0;
>>>>     CID 1313836:  Null pointer dereferences  (FORWARD_NULL)
>>>>     Assigning: "rr_dir->status" = "NULL".
> 150                     rr_dir->status = NULL;
> 151                     pos = -1 - pos;
> 152
> 153                     /* Make sure the array is big enough ... */
> 154                     ALLOC_GROW(rerere_dir, rerere_dir_nr + 1,
> rerere_dir_alloc);
> 155                     /* ... and add it in. */
>
> ** CID 1313835:  Null pointer dereferences  (FORWARD_NULL)
> /builtin/fetch.c: 795 in prune_refs()
> --
> Duy
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New Defects reported by Coverity Scan for git

[Stefan Beller]

On Fri, Jul 31, 2015 at 9:11 AM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Jul 31, 2015 at 4:24 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> Jeff, I suppose you are the admin of git on scan.coverity, or knows
>> him/her, perhaps we can add a model for xmalloc to suppress these
>> "null pointer deferences" reports? We are sure xmalloc() never returns
>> NULL. Qemu did it [1] and it looks simple.. I think something like
>> this would do
>>
>> void *xmalloc(size_t size)
>> {
>>    void *mem = malloc(size);
>>    if (!mem) __coverity_panic__();
>>    return mem;
>> }
>>
>> [1] http://git.qemu.org/?p=qemu.git;a=blob;f=scripts/coverity-model.c;h=4c99a85cfc292caa9edd9d041e2683ee53490a8d;hb=e40cdb0e6efb795e4d19368987d53e3e4ae19cf7#l104
>>
>
> Taking just that excerpt doesn't work. Upload fails with
> "modeling_file.c", line 12: error #20:
>           identifier "malloc" is undefined
>   void *mem = malloc(size);
>
> I'll look into your reference[1] a bit more and try to follow it as a guidance.

So I put in these lines into the modeling file:
void *malloc(size_t);
void *calloc(size_t, size_t);
void *realloc(void *, size_t);
void free(void *);


void *xrealloc(void *ptr, size_t size)
{
  void *ret = realloc(ptr, size);
  if (!ret) __coverity_panic__();
  return ret;
}

void *xmalloc(size_t size)
{
  void *mem = malloc(size);
  if (!mem) __coverity_panic__();
  return mem;
}

void xcalloc(size_t num, size_t size)
{
  void *ret = calloc(num, size);
  if (!ret)  __coverity_panic__();
  return ret;
}

and there seem to be 42 new defects and 20 fixed defects by the modeling of
memory allocations. We'd need to check if coverity understood the modeling
as we intended it. Looking at the first few issues, they seem to be
correctly finding
leaks.


>
>
>>
>> ---------- Forwarded message ----------
>> From:  <scan-admin@coverity.com>
>> Date: Fri, Jul 31, 2015 at 5:54 PM
>> Subject: New Defects reported by Coverity Scan for git
>> To: pclouds@gmail.com
>>
>> _______________________________________________________________________________________________________
>> *** CID 1313836:  Null pointer dereferences  (FORWARD_NULL)
>> /rerere.c: 150 in find_rerere_dir()
>> 144                     return NULL; /* BUG */
>> 145             pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr,
>> rerere_dir_sha1);
>> 146             if (pos < 0) {
>> 147                     rr_dir = xmalloc(sizeof(*rr_dir));
>> 148                     hashcpy(rr_dir->sha1, sha1);
>> 149                     rr_dir->status_nr = rr_dir->status_alloc = 0;
>>>>>     CID 1313836:  Null pointer dereferences  (FORWARD_NULL)
>>>>>     Assigning: "rr_dir->status" = "NULL".
>> 150                     rr_dir->status = NULL;
>> 151                     pos = -1 - pos;
>> 152
>> 153                     /* Make sure the array is big enough ... */
>> 154                     ALLOC_GROW(rerere_dir, rerere_dir_nr + 1,
>> rerere_dir_alloc);
>> 155                     /* ... and add it in. */
>>
>> ** CID 1313835:  Null pointer dereferences  (FORWARD_NULL)
>> /builtin/fetch.c: 795 in prune_refs()
>> --
>> Duy
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fwd: New Defects reported by Coverity Scan for git

[Duy Nguyen]

I think Coverity caught this correctly.

** CID 1306846:  Memory - illegal accesses  (USE_AFTER_FREE)
/builtin/pull.c: 287 in config_get_rebase()


________________________________________________________________________________________________________
*** CID 1306846:  Memory - illegal accesses  (USE_AFTER_FREE)
/builtin/pull.c: 287 in config_get_rebase()
281
282             if (curr_branch) {
283                     char *key = xstrfmt("branch.%s.rebase",
curr_branch->name);
284
285                     if (!git_config_get_value(key, &value)) {
286                             free(key);
>>>     CID 1306846:  Memory - illegal accesses  (USE_AFTER_FREE)
>>>     Passing freed pointer "key" as an argument to "parse_config_rebase".
287                             return parse_config_rebase(key, value, 1);
288                     }
289
290                     free(key);
291             }
292
-- 
Duy

Re: New Defects reported by Coverity Scan for git

[Paul Tan]

On Wed, Jun 17, 2015 at 9:54 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> I think Coverity caught this correctly.
>
> ** CID 1306846:  Memory - illegal accesses  (USE_AFTER_FREE)
> /builtin/pull.c: 287 in config_get_rebase()
>
>
> ________________________________________________________________________________________________________
> *** CID 1306846:  Memory - illegal accesses  (USE_AFTER_FREE)
> /builtin/pull.c: 287 in config_get_rebase()
> 281
> 282             if (curr_branch) {
> 283                     char *key = xstrfmt("branch.%s.rebase",
> curr_branch->name);
> 284
> 285                     if (!git_config_get_value(key, &value)) {
> 286                             free(key);
>>>>     CID 1306846:  Memory - illegal accesses  (USE_AFTER_FREE)
>>>>     Passing freed pointer "key" as an argument to "parse_config_rebase".
> 287                             return parse_config_rebase(key, value, 1);
> 288                     }
> 289
> 290                     free(key);
> 291             }
> 292

Ugh, thanks. ><

Regards,
Paul