threads / discuss / 39979

Fwd: New Defects reported by Coverity Scan for git

Subject: Fwd: New Defects reported by Coverity Scan for git

## tl;dr

3 messages between Jul 31, 2015 and Jul 31, 2015.

replies: 2people: 2as markdown or json

Duy Nguyen· Jul 31, 2015, 11:24 UTC · lore

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

Stefan Beller· Jul 31, 2015, 16:11 UTC · re: Duy Nguyen · lore

Re: New Defects reported by Coverity Scan for git

On Fri, Jul 31, 2015 at 4:24 AM, Duy Nguyen <pclouds@gmail.com> wrote:

Show 15 quoted lines

> 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.

Show 35 quoted lines

>
> ---------- 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

Stefan Beller· Jul 31, 2015, 16:58 UTC · re: Stefan Beller · lore

Re: New Defects reported by Coverity Scan for git

On Fri, Jul 31, 2015 at 9:11 AM, Stefan Beller <sbeller@google.com> wrote:

Show 23 quoted lines

> 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.

Show 37 quoted lines

>
>
>>
>> ---------- 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