Re: New Defects reported by Coverity Scan for RTEMS-Newlib

[Jeff Johnston <jjohnstn@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 14762 bytes --]

Hi Joel, I will pushing a patch shortly but see comments below as IMO there
are some false positives.

-- Jeff J.

On Tue, Aug 30, 2022 at 3:03 PM Jeff Johnston <jjohnstn@redhat.com> wrote:

> Thanks Joel for bringing this to our attention.
>
> -- Jeff J.
>
> On Mon, Aug 29, 2022 at 7:09 PM Joel Sherrill <joel@rtems.org> wrote:
>
>> Hi
>>
>> I quit running Coverity on newlib as part of the repositories analysed as
>> part of RTEMS BUT I had to update the version of cov-analysis we used and
>> wanted to make sure the scripting stayed working.
>>
>> These issues were flagged since the last time we ran it. Some look like
>> they need attention.
>>
>> --joel
>>
>> ---------- Forwarded message ---------
>> From: <scan-admin@coverity.com>
>> Date: Mon, Aug 29, 2022 at 5:56 PM
>> Subject: New Defects reported by Coverity Scan for RTEMS-Newlib
>> To: <joel.sherrill@gmail.com>
>>
>>
>> Hi,
>>
>> Please find the latest report on new defect(s) introduced to RTEMS-Newlib
>> found with Coverity Scan.
>>
>> 10 new defect(s) introduced to RTEMS-Newlib found with Coverity Scan.
>> 1 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 10 of 10 defect(s)
>>
>>
>> ** CID 398779:    (UNINIT)
>>
>>
>> Fixed.


>
>> ________________________________________________________________________________________________________
>> *** CID 398779:    (UNINIT)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdio/swscanf.c:
>> 454 in _swscanf_r()
>> 448       f._bf._size = f._r = wcslen (str) * sizeof (wchar_t);
>> 449       f._read = __seofread;
>> 450       f._ub._base = NULL;
>> 451       f._lb._base = NULL;
>> 452       f._file = -1;  /* No file. */
>> 453       va_start (ap, fmt);
>> >>>     CID 398779:    (UNINIT)
>> >>>     Using uninitialized value "f._flags2" when calling
>> "__ssvfwscanf_r".
>> 454       ret = __ssvfwscanf_r (ptr, &f, fmt, ap);
>> 455       va_end (ap);
>> 456       return ret;
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdio/swscanf.c:
>> 454 in _swscanf_r()
>> 448       f._bf._size = f._r = wcslen (str) * sizeof (wchar_t);
>> 449       f._read = __seofread;
>> 450       f._ub._base = NULL;
>> 451       f._lb._base = NULL;
>> 452       f._file = -1;  /* No file. */
>> 453       va_start (ap, fmt);
>> >>>     CID 398779:    (UNINIT)
>> >>>     Using uninitialized value "f._ur" when calling "__ssvfwscanf_r".
>> 454       ret = __ssvfwscanf_r (ptr, &f, fmt, ap);
>> 455       va_end (ap);
>> 456       return ret;
>>
>> ** CID 398778:  High impact quality  (Y2K38_SAFETY)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/posix/sleep.c:
>> 18 in sleep()
>>
>>
Fixed.  (I anded with UINT_MAX so the checker should allow this).

>
>>
>> ________________________________________________________________________________________________________
>> *** CID 398778:  High impact quality  (Y2K38_SAFETY)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/posix/sleep.c:
>> 18 in sleep()
>> 12     {
>> 13         struct timespec ts;
>> 14
>> 15         ts.tv_sec = seconds;
>> 16         ts.tv_nsec = 0;
>> 17         if (!nanosleep(&ts,&ts)) return 0;
>> >>>     CID 398778:  High impact quality  (Y2K38_SAFETY)
>> >>>     A "time_t" value is stored in an integer with too few bits to
>> accommodate it.  The expression "ts.tv_sec" is cast to "unsigned int".
>> 18         if (errno == EINTR) return ts.tv_sec;
>> 19         return -1;
>> 20     }
>> 21
>>
>> ** CID 398777:    (UNINIT)
>>
>>
>> Fixed.  I just initialized the fields.


>
>> ________________________________________________________________________________________________________
>> *** CID 398777:    (UNINIT)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdio/swscanf.c:
>> 432 in swscanf()
>> 426       f._bf._size = f._r = wcslen (str) * sizeof (wchar_t);
>> 427       f._read = __seofread;
>> 428       f._ub._base = NULL;
>> 429       f._lb._base = NULL;
>> 430       f._file = -1;  /* No file. */
>> 431       va_start (ap, fmt);
>> >>>     CID 398777:    (UNINIT)
>> >>>     Using uninitialized value "f._flags2" when calling
>> "__ssvfwscanf_r".
>> 432       ret = __ssvfwscanf_r (_REENT, &f, fmt, ap);
>> 433       va_end (ap);
>> 434       return ret;
>> 435     }
>> 436
>> 437     #endif /* !_REENT_ONLY */
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdio/swscanf.c:
>> 432 in swscanf()
>> 426       f._bf._size = f._r = wcslen (str) * sizeof (wchar_t);
>> 427       f._read = __seofread;
>> 428       f._ub._base = NULL;
>> 429       f._lb._base = NULL;
>> 430       f._file = -1;  /* No file. */
>> 431       va_start (ap, fmt);
>> >>>     CID 398777:    (UNINIT)
>> >>>     Using uninitialized value "f._ur" when calling "__ssvfwscanf_r".
>> 432       ret = __ssvfwscanf_r (_REENT, &f, fmt, ap);
>> 433       va_end (ap);
>> 434       return ret;
>> 435     }
>> 436
>> 437     #endif /* !_REENT_ONLY */
>>
>> ** CID 398776:    (UNINIT)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/time/time.c:
>> 44 in time()
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/time/time.c:
>> 45 in time()
>>
>>
>> Although this should be a false positive because of gettimeofday_r
getting the address of now, I have
initialized the field to -1.


>
>> ________________________________________________________________________________________________________
>> *** CID 398776:    (UNINIT)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/time/time.c:
>> 44 in time()
>> 38       struct timeval now;
>> 39
>> 40       if (_gettimeofday_r (_REENT, &now, NULL) < 0)
>> 41         now.tv_sec = (time_t) -1;
>> 42
>> 43       if (t)
>> >>>     CID 398776:    (UNINIT)
>> >>>     Using uninitialized value "now.tv_sec".
>> 44         *t = now.tv_sec;
>> 45       return now.tv_sec;
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/time/time.c:
>> 45 in time()
>> 39
>> 40       if (_gettimeofday_r (_REENT, &now, NULL) < 0)
>> 41         now.tv_sec = (time_t) -1;
>> 42
>> 43       if (t)
>> 44         *t = now.tv_sec;
>> >>>     CID 398776:    (UNINIT)
>> >>>     Using uninitialized value "now.tv_sec".
>> 45       return now.tv_sec;
>>
>> ** CID 398775:    (UNINIT)
>>
>>
>>
Fixed.  I initialized the fields.

>
>> ________________________________________________________________________________________________________
>> *** CID 398775:    (UNINIT)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdio/vswscanf.c:
>> 57 in _vswscanf_r()
>> 51       f._bf._base = f._p = (unsigned char *) str;
>> 52       f._bf._size = f._r = wcslen (str) * sizeof (wchar_t);
>> 53       f._read = __seofread;
>> 54       f._ub._base = NULL;
>> 55       f._lb._base = NULL;
>> 56       f._file = -1;  /* No file. */
>> >>>     CID 398775:    (UNINIT)
>> >>>     Using uninitialized value "f._ur" when calling "__ssvfwscanf_r".
>> 57       return __ssvfwscanf_r (ptr, &f, fmt, ap);
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdio/vswscanf.c:
>> 57 in _vswscanf_r()
>> 51       f._bf._base = f._p = (unsigned char *) str;
>> 52       f._bf._size = f._r = wcslen (str) * sizeof (wchar_t);
>> 53       f._read = __seofread;
>> 54       f._ub._base = NULL;
>> 55       f._lb._base = NULL;
>> 56       f._file = -1;  /* No file. */
>> >>>     CID 398775:    (UNINIT)
>> >>>     Using uninitialized value "f._flags2" when calling
>> "__ssvfwscanf_r".
>> 57       return __ssvfwscanf_r (ptr, &f, fmt, ap);
>>
>> ** CID 398774:  Uninitialized variables  (UNINIT)
>>
>> Fixed. I memset the initial array to 0's.


>
>>
>> ________________________________________________________________________________________________________
>> *** CID 398774:  Uninitialized variables  (UNINIT)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/stdlib/arc4random.c:
>> 93 in _rs_stir()
>> 87      u_char rnd[KEYSZ + IVSZ];
>> 88
>> 89      if (getentropy(rnd, sizeof rnd) == -1)
>> 90              _getentropy_fail();
>> 91
>> 92      if (!rs)
>> >>>     CID 398774:  Uninitialized variables  (UNINIT)
>> >>>     Using uninitialized element of array "rnd" when calling
>> "_rs_init".
>> 93              _rs_init(rnd, sizeof(rnd));
>> 94      else
>> 95              _rs_rekey(rnd, sizeof(rnd));
>> 96      explicit_bzero(rnd, sizeof(rnd));       /* discard source seed */
>> 97
>> 98      /* invalidate rs_buf */
>>
>> ** CID 398773:  Incorrect expression  (DIVIDE_BY_ZERO)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libm/math/kf_tan.c:
>> 55 in __kernel_tanf()
>>
>>
>>
This is a false positive.  We intend to divide by zero.

>
>> ________________________________________________________________________________________________________
>> *** CID 398773:  Incorrect expression  (DIVIDE_BY_ZERO)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libm/math/kf_tan.c:
>> 55 in __kernel_tanf()
>> 49      __int32_t ix,hx;
>> 50      GET_FLOAT_WORD(hx,x);
>> 51      ix = hx&0x7fffffff;     /* high word of |x| */
>> 52      if(ix<0x31800000)                       /* x < 2**-28 */
>> 53          {if((int)x==0) {                    /* generate inexact */
>> 54              if((ix|(iy+1))==0) return one/fabsf(x);
>> >>>     CID 398773:  Incorrect expression  (DIVIDE_BY_ZERO)
>> >>>     In expression "-1f / x", division by expression "x" which may be
>> zero has undefined behavior.
>> 55              else return (iy==1)? x: -one/x;
>> 56          }
>> 57          }
>> 58      if(ix>=0x3f2ca140) {                    /* |x|>=0.6744 */
>> 59          if(hx<0) {x = -x; y = -y;}
>> 60          z = pio4-x;
>>
>> ** CID 398772:  Memory - corruptions  (OVERRUN)
>>
>> I think this is another false positive.  I couldn't see where it was
accessing storage without doing a length check
first.


>
>>
>> ________________________________________________________________________________________________________
>> *** CID 398772:  Memory - corruptions  (OVERRUN)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/posix/regcomp.c:
>> 1044 in bothcases()
>> 1038            assert(othercase(ch) != ch);    /* p_bracket() would
>> recurse */
>> 1039            p->next = bracket;
>> 1040            p->end = bracket+2;
>> 1041            bracket[0] = ch;
>> 1042            bracket[1] = ']';
>> 1043            bracket[2] = '\0';
>> >>>     CID 398772:  Memory - corruptions  (OVERRUN)
>> >>>     Overrunning buffer pointed to by "p->next" of 3 bytes by passing
>> it
>> to a function which accesses it at byte offset 4.
>> 1044            p_bracket(p);
>> 1045            assert(p->next == bracket+2);
>> 1046            p->next = oldnext;
>> 1047            p->end = oldend;
>> 1048     }
>> 1049
>>
>> ** CID 398771:  High impact quality  (Y2K38_SAFETY)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/posix/usleep.c:
>> 18 in usleep()
>>
>>
Fixed.  I removed the return as usleep is supposed to return -1 on an EINTR.


>>
>> ________________________________________________________________________________________________________
>> *** CID 398771:  High impact quality  (Y2K38_SAFETY)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/posix/usleep.c:
>> 18 in usleep()
>> 12     {
>> 13         struct timespec ts;
>> 14
>> 15         ts.tv_sec = (long int)useconds / 1000000;
>> 16         ts.tv_nsec = ((long int)useconds % 1000000) * 1000;
>> 17         if (!nanosleep(&ts,&ts)) return 0;
>> >>>     CID 398771:  High impact quality  (Y2K38_SAFETY)
>> >>>     A "time_t" value is stored in an integer with too few bits to
>> accommodate it.  The expression "ts.tv_sec" is cast to "int".
>> 18         if (errno == EINTR) return ts.tv_sec;
>> 19         return -1;
>> 20     }
>> 21
>>
>> ** CID 378851:  Memory - corruptions  (OVERRUN)
>>
>>
Again, I believe this is a false positive.  No access of storage without
checking length first.

>
>>
>> ________________________________________________________________________________________________________
>> *** CID 378851:  Memory - corruptions  (OVERRUN)
>>
>> /home/joel/rtems-cron-coverity/sourceware-mirror-newlib-cygwin/newlib/libc/posix/regcomp.c:
>> 1090 in nonnewline()
>> 1084            p->next = bracket;
>> 1085            p->end = bracket+3;
>> 1086            bracket[0] = '^';
>> 1087            bracket[1] = '\n';
>> 1088            bracket[2] = ']';
>> 1089            bracket[3] = '\0';
>> >>>     CID 378851:  Memory - corruptions  (OVERRUN)
>> >>>     Overrunning buffer pointed to by "p->next" of 4 bytes by passing
>> it
>> to a function which accesses it at byte offset 4.
>> 1090            p_bracket(p);
>> 1091            assert(p->next == bracket+3);
>> 1092            p->next = oldnext;
>> 1093            p->end = oldend;
>> 1094     }
>> 1095
>>
>>
>>
>> ________________________________________________________________________________________________________
>> To view the defects in Coverity Scan visit,
>>
>> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50ypUUzi-2FdSNmuyRB7BEFT8xQWqa-2BcrUOdcmLJRN5wHA-2F-2Bj-2BUPxOS2vpJc2U7lnvDDSM-3DgcXN_CTvEjVoKhyc6dLmJJo1u9AYIk8P8bcAbCPbBDYvYSXrko-2B6zqtxlihMO5pRBlqs6CXC6JoeSQ5BknttytYW4gn54pXoG5E1T2VTg7ZExldrWnOHoGNfjITpyeGBnq8zf1R1SvLaQHX0KwLC3QLIILHDIyeRDmH6ivilCfFIJbx4IaHchThYPPrH23evm0vJ6A6-2BcYCz2qmJNN2577UqVyYc0aItJ859abhW8GanEpsc-3D
>>
>>   To manage Coverity Scan email notifications for "
>> joel.sherrill@gmail.com",
>> click
>>
>> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxTJDdEZ5ceQ-2BXdf-2FM1tcMIXP73MN3HxQfFTMLU5dSe8Rv0KFh7gYStOFjZD12ucRRnrjyUHOCTj7rG0E9HBcwa6j-2FX4NTabdEq2v7MM-2FuqaA-3D5Xsf_CTvEjVoKhyc6dLmJJo1u9AYIk8P8bcAbCPbBDYvYSXrko-2B6zqtxlihMO5pRBlqs6CXC6JoeSQ5BknttytYW4gsEM86eEaAqPEjIHUArLBXYOUpWfZ4bmwC96PG11GPPh-2FLsC0rkTKQE2J8XRI45hCnTpCTbj87kq0GI1XLddKyw1JXGGqDcyizThGumwZmd8Tr5waHqdorDd3Wom83BYSMOhcHiGVjpnvscbd8ReGFw-3D
>>
>>