Bug 2047926

Possibly related is this additional problem:

[root@rawhide-efi ~]# cat test.c
#define _GNU_SOURCE

#include <poll.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/un.h>

int main() {
    struct pollfd *pollfds, *newpollfds;
    struct sockaddr_un remote;
    socklen_t len = sizeof(remote);

    pollfds = calloc(1, sizeof(struct pollfd));
    if (!pollfds)
	return 1;

    pollfds[0].fd = 1;

    newpollfds = realloc(pollfds, 2 * sizeof(struct pollfd));
    if (!newpollfds)
	return 1;

    accept(pollfds[0].fd, &remote, &len);

    return 0;
}
[root@rawhide-efi ~]# gcc -O3 -fanalyzer test.c
test.c: In function ‘main’:
test.c:24:5: warning: use after ‘free’ of ‘pollfds’ [CWE-416] [-Wanalyzer-use-after-free]
   24 |     accept(pollfds[0].fd, &remote, &len);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘main’: events 1-9
    |
    |   14 |     pollfds = calloc(1, sizeof(struct pollfd));
    |      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (1) allocated here
    |   15 |     if (!pollfds)
    |      |        ~       
    |      |        |
    |      |        (2) assuming ‘pollfds’ is non-NULL
    |      |        (3) following ‘false’ branch (when ‘pollfds’ is non-NULL)...
    |......
    |   18 |     pollfds[0].fd = 1;
    |      |     ~~~~~~~~~~~~~~~~~
    |      |                   |
    |      |                   (4) ...to here
    |   19 | 
    |   20 |     newpollfds = realloc(pollfds, 2 * sizeof(struct pollfd));
    |      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (5) freed here
    |      |                  (6) when ‘realloc’ succeeds, moving buffer
    |   21 |     if (!newpollfds)
    |      |        ~       
    |      |        |
    |      |        (7) following ‘false’ branch (when ‘newpollfds’ is non-NULL)...
    |......
    |   24 |     accept(pollfds[0].fd, &remote, &len);
    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |     |
    |      |     (8) ...to here
    |      |     (9) use after ‘free’ of ‘pollfds’; freed at (5)
    |
[root@rawhide-efi ~]# 

(Source continues to be pesign.)

(In reply to Robbie Harwood from comment #1)

The report in comment #1 looks to me like the analyzer is correctly reporting a bug in test.c: if realloc succeeds but moves the buffer, then the existing "pollfds" points to freed memory.

(In reply to Robbie Harwood from comment #0)
Thanks for filing this; I think the false positive in comment #0 is due to a bad interaction with attributes in the glibc headers.  Please can you attach the preprocessed source (via -E)

Created attachment 1858170 [details]
reallocarray analyzer problem - processed

Attached.

Apologies about the second one - I seem to have minimized it poorly.  I'll see if I can get something better out.

Created attachment 1858172 [details]
similar realloc problem - processed

Here's that second one, hopefully fixed :)  Source this time is:

#define _GNU_SOURCE

#include <poll.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/un.h>
#include <unistd.h>

int main() {
    int rc;
    int nsockets = 1;
    struct pollfd *pollfds, *newpollfds;
    struct sockaddr_un remote;
    socklen_t len = sizeof(remote);

    pollfds = calloc(1, sizeof(struct pollfd));
    if (!pollfds) {
        exit(1);
    }

    while (1) {
        rc = ppoll(pollfds, nsockets, NULL, NULL);
        if (rc < 0) {
            continue;
        }

        if (pollfds[0].revents & POLLIN) {
            nsockets++;
	    newpollfds = realloc(pollfds, nsockets * sizeof(*pollfds));
            if (!newpollfds) {
                exit(1);
            }
            pollfds = newpollfds;
            pollfds[nsockets-1].fd = accept(pollfds[0].fd, &remote,
					    &len);
        }
    }
    return 0;
}

(In reply to Robbie Harwood from comment #4)
> Created attachment 1858170 [details]
> reallocarray analyzer problem - processed
> 
> Attached.

Thanks; I can reproduce the false positive with that one.

Looking at attachment 1858170 [details], I see:

# 396 "/usr/include/stdlib.h" 2 3 4

followed by various parts of your <stdlib.h>:

[...snip...]

extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
     __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__warn_unused_result__))
     __attribute__ ((__alloc_size__ (2, 3)))
    __attribute__ ((__malloc__ (__builtin_free, 1)));

extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
     __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__malloc__ (reallocarray, 1)));


where there seem to be two declarations of reallocarray, with two different sets of attributes.  If I delete the 2nd, the false positive goes away.

This seems to be from:
  https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=c1760eaf3b575ad174fd88b252fd16bd525fa818

Martin: why is reallocarray declared twice?  Is it due to wanting to declare two different deallocators?  Looks like our implementations might disagree about the semantics of this attribute (bother)

The first declaration just forward-declares reallocarray (and associates it with free).  The second one then associates it with itself as its own deallocator.  The two steps are necessary because a declaration needs to be in scope when it's mentioned in an attribute, and one's not yet there when attributes on the first one are being processed.

(In reply to Robbie Harwood from comment #5)
> Created attachment 1858172 [details]
> similar realloc problem - processed
> 
> Here's that second one, hopefully fixed :)  Source this time is:

[...snip...]

Thanks.  I don't see a  "use after ‘free’ of ‘pollfds" with that reproducer, but I do see a pair of bogus "-Wanalyzer-use-of-uninitialized-value" warnings.  This seems to be a separate issue from the reallocarray issue, so I've filed the bogus uninit warnings upstream as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104369 and will try to fix them there.

(In reply to Martin Sebor from comment #7)
> The first declaration just forward-declares reallocarray (and associates it
> with free).  The second one then associates it with itself as its own
> deallocator.  The two steps are necessary because a declaration needs to be
> in scope when it's mentioned in an attribute, and one's not yet there when
> attributes on the first one are being processed.

Thanks.  I've filed this issue as an analyzer bug with upstream GCC here:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104370

(In reply to Dave Malcolm from comment #8)
> (In reply to Robbie Harwood from comment #5)
> > Created attachment 1858172 [details]
> > similar realloc problem - processed
> > 
> > Here's that second one, hopefully fixed :)  Source this time is:
> 
> [...snip...]
> 
> Thanks.  I don't see a  "use after ‘free’ of ‘pollfds" with that reproducer,
> but I do see a pair of bogus "-Wanalyzer-use-of-uninitialized-value"
> warnings.  This seems to be a separate issue from the reallocarray issue, so
> I've filed the bogus uninit warnings upstream as
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104369 and will try to fix them
> there.

I've fixed the "uninit on realloc" bug upstream in commit r12-7040-g3ef328c293a336df0aead2d72c0c5ed9781a9861:
  https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=3ef328c293a336df0aead2d72c0c5ed9781a9861

FEDORA-2022-fba9d09417 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-fba9d09417

FEDORA-2022-fba9d09417 has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.