On Tue, Sep 15, 2020 at 11:08:04AM +0200, Martijn van Duren wrote:
> There are 3 things that actually look like valid complaints when running
> clang's static analyzer.
>
> 1) A dead store in agentx_recv.
> 2) sizeof(ipaddress) intead of sizeof(*ipaddress). Since this is ipv4,
> this is only a problem if sizeof(pointer) is smaller then 4 bytes,
> which can't happen afaik.
> 3) dst does indeed need to be dereffed, but since dstlen and buflen are
> initialized to 0 and srclen is practically always larger then 0
> we're fine. I'm keeping the *dst here as an additional safeguard.
>
> The rest seem like false positives to me.
>
> OK?
>
> martijn@
>
> Index: agentx.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/agentx.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 agentx.c
> --- agentx.c 14 Sep 2020 11:30:25 -0000 1.16
> +++ agentx.c 15 Sep 2020 09:05:57 -0000
> @@ -135,7 +135,6 @@ agentx_recv(struct agentx *ax)
> header.aph_packetid = agentx_pdutoh32(&header, u8);
> u8 += 4;
> header.aph_plength = agentx_pdutoh32(&header, u8);
> - u8 += 4;
>
> if (header.aph_version != 1) {
> errno = EPROTO;
ok for this piece above
> Index: subagentx.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 subagentx.c
> --- subagentx.c 14 Sep 2020 11:30:25 -0000 1.1
> +++ subagentx.c 15 Sep 2020 09:05:58 -0000
> @@ -2929,7 +2929,7 @@ getnext:
> index->sav_idatacomplete = 1;
> break;
> case AGENTX_DATA_TYPE_IPADDRESS:
> - ipaddress = calloc(1, sizeof(ipaddress));
> + ipaddress = calloc(1, sizeof(*ipaddress));
> if (ipaddress == NULL) {
> subagentx_log_sag_warn(sag,
> "Failed to bind ipaddress index");
Not ok for this, it's probably correct, but there are other instances of this
in this code and so you need to engage brain, not static analyzer, go fix or
don't fix them all in a separate commit.
> @@ -3833,7 +3833,7 @@ subagentx_strcat(char **dst, const char
> }
>
> srclen = strlen(src);
> - if (dst == NULL || dstlen + srclen > buflen) {
> + if (*dst == NULL || dstlen + srclen > buflen) {
> nbuflen = (((dstlen + srclen) / 512) + 1) * 512;
> tmp = recallocarray(*dst, buflen, nbuflen, sizeof(*tmp));
> if (tmp == NULL)
>
ok for this piece above
