classification
Title: socketmodule.c: _FORTIFY_SOURCE=2 warning in AF_ALG case of getsockaddrarg()
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, pablogsal, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2019-09-26 01:11 by vstinner, last changed 2019-10-14 09:51 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16698 merged vstinner, 2019-10-10 14:35
PR 16706 closed miss-islington, 2019-10-10 19:30
PR 16738 merged pablogsal, 2019-10-13 00:19
Messages (7)
msg353251 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-26 01:11
To test bpo-32375, I just recompiled the master branch of Python twice using these flags:

* -D_FORTIFY_SOURCE=2 -Og
* -D_FORTIFY_SOURCE=2 -O3

I got a few warnings, the same that I get without FORTIFY SOURCE.

Using -D_FORTIFY_SOURCE=2 -O3, I get one warning, but it's no longer from getpath.c:

In file included from /usr/include/string.h:494,
                 from ./Include/Python.h:30,
                 from /home/vstinner/python/master/Modules/socketmodule.c:103:
In function ‘memset’,
    inlined from ‘getsockaddrarg’ at /home/vstinner/python/master/Modules/socketmodule.c:2331:9,
    inlined from ‘sock_bind’ at /home/vstinner/python/master/Modules/socketmodule.c:3113:10:
/usr/include/bits/string_fortified.h:71:10: warning: ‘__builtin_memset’ offset [17, 88] from the object at ‘addrbuf’ is out of the bounds of referenced subobject ‘sa’ with type ‘struct sockaddr’ at offset 0 [-Warray-bounds]
   71 |   return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The warning comes from the code:

    case AF_ALG:
        ...
        struct sockaddr_alg *sa;
        sa = (struct sockaddr_alg *)addr_ret;
        memset(sa, 0, sizeof(*sa));

called from:

static PyObject *
sock_bind(PySocketSockObject *s, PyObject *addro)
{
    sock_addr_t addrbuf;
    int addrlen;
    int res;

    if (!getsockaddrarg(s, addro, SAS2SA(&addrbuf), &addrlen, "bind")) {
        return NULL;
    }
msg353561 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-30 10:15
Similar when building Python on Fedora 30 (GCC version 9.2.1) with gcc -O3:

In function ‘getsockaddrarg’,
    inlined from ‘sock_bind’ at /home/vstinner/python/master/Modules/socketmodule.c:3113:10:
/home/vstinner/python/master/Modules/socketmodule.c:2331:9: warning: ‘memset’ offset [17, 88] from the object at ‘addrbuf’ is out of the bounds of referenced subobject ‘sa’ with type ‘struct sockaddr’ at offset 0 [-Warray-bounds]
 2331 |         memset(sa, 0, sizeof(*sa));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
msg354400 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-10 19:30
New changeset d565fb9828ee9c494bb7a80057a08e4738273e30 by Victor Stinner in branch 'master':
bpo-38282: Rewrite getsockaddrarg() helper function (GH-16698)
https://github.com/python/cpython/commit/d565fb9828ee9c494bb7a80057a08e4738273e30
msg354556 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-10-12 23:07
The failures on FreeBSD are:

/usr/home/buildbot/python/3.x.koobs-freebsd10.nondebug/build/Modules/socketmodule.c:1931:50: error: no member named 'bt_l2' in 'union sock_addr'
            struct sockaddr_l2 *addr = &addrbuf->bt_l2;
                                        ~~~~~~~  ^
/usr/home/buildbot/python/3.x.koobs-freebsd10.nondebug/build/Modules/socketmodule.c:1950:50: error: no member named 'bt_rc' in 'union sock_addr'
            struct sockaddr_rc *addr = &addrbuf->bt_rc;
                                        ~~~~~~~  ^
/usr/home/buildbot/python/3.x.koobs-freebsd10.nondebug/build/Modules/socketmodule.c:1967:51: error: no member named 'bt_hci' in 'union sock_addr'
            struct sockaddr_hci *addr = &addrbuf->bt_hci;
                                         ~~~~~~~  ^
3 errors generated.

This is likely due to the fact that those fields are protected by:

typedef union sock_addr {
#ifdef HAVE_BLUETOOTH_BLUETOOTH_H
    struct sockaddr_l2 bt_l2;
    struct sockaddr_rc bt_rc;
    struct sockaddr_sco bt_sco;
    struct sockaddr_hci bt_hci;
#elif defined(MS_WINDOWS)
}

and in those machines HAVE_BLUETOOTH_BLUETOOTH_H is absent.
msg354559 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-10-13 00:34
PR 16738 fixes the FreeBSD buildbots by correctly handling the Bluetooth L2CAP socket structure in FreeBSD if only bluetooth.h is available. Check out https://man.cx/ng_btsocket(4) for example for more info in the ng_btsocket protocol for FreeBSD.
msg354562 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-10-13 01:03
New changeset 27b33fb41a7c64a6211d73d14804aa0cd6defccb by Pablo Galindo in branch 'master':
bpo-38282: Correctly manage the Bluetooth L2CAP socket structure in FreeBSD (GH-16738)
https://github.com/python/cpython/commit/27b33fb41a7c64a6211d73d14804aa0cd6defccb
msg354623 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-14 09:51
I close the issue.

I planned to backport the change to 3.8, but 3.8.0 final is supposed to be released today. I prefer to avoid putting a last minute regression on some platforms like DragonflyBSD in 3.8. Backporting the change is not strictly required, it's just a compiler warning, it's a false alarm, and it's only when building with _FORTIFY_SOURCE defined.
History
Date User Action Args
2019-10-14 09:51:29vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg354623

stage: patch review -> resolved
2019-10-13 01:03:58pablogsalsetmessages: + msg354562
2019-10-13 00:34:11pablogsalsetmessages: + msg354559
2019-10-13 00:19:30pablogsalsetpull_requests: + pull_request16317
2019-10-12 23:07:54pablogsalsetnosy: + pablogsal
messages: + msg354556
2019-10-10 19:30:31miss-islingtonsetpull_requests: + pull_request16291
2019-10-10 19:30:23vstinnersetmessages: + msg354400
2019-10-10 14:35:33vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request16285
2019-09-30 10:15:51vstinnersetmessages: + msg353561
2019-09-26 01:11:45vstinnercreate