[sssd PR#740][opened] sbus: avoid using invalid stack point in SBUS_INTERFACE

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/740
Author: pbrezina
 Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE
Action: opened

PR body:
"""
SBUS_INTERFACE macros expanded as:
struct sbus_interface bus =
    ({ sbus_interface(
        "org.freedesktop.DBus",
        ((void *)0),
        (((const struct sbus_method[])
        {
            ({
                /* ... compile time check of function signature omitted */ ;
                sbus_method_sync(/* ... full list of params omitted */);
            }),
  ...

This however includes an issue that methods/properties/signals are returned
by value, however stored in sbus_interface as pointers. Once we return out
of the top-level block and assign resulting sbus_interface into 'bus' variable
those objects allocated on stack becomes invalid and can be overwritten by other
allocations on stack.

This patch overcomes this issue by changing declaration of SBUS_INTERFACE and
avoiding using this top-level block. This still keeps the declarative structure
and simplifies the code as it does not require any memory handling and
tests for successful allocations.

const struct sbus_method __ ## varname ## _m[] = methods;                 \
    const struct sbus_signal __ ## varname ## _s[] = signals;                 \
    const struct sbus_property __ ## varname ## _p[] = properties;            \
    struct sbus_interface varname = SBUS_IFACE_ ## iface(                     \
        (__ ## varname ## _m),                                                \
        (__ ## varname ## _s),                                                \
        (__ ## varname ## _p)                                                 \
    )

Resolves:
https://pagure.io/SSSD/sssd/issue/3924
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/740/head:pr740
git checkout pr740

[sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE

alexey-tikhonov commented:
"""
> * Consider following snippet:
>     struct sbus_interface dbus;
>     {
>         /* ... some code ... */
>         SBUS_INTERFACE(tmp, ...);
>         /* ... some code ... */
>         dbus = tmp;
>     }
>     /* do something with dbus, that contain pointers to variables went out of scope */
>
> While I can't give a good reason to write code this way, still it _may seem_ to be valid, taking into account that `struct sbus_interface` contains only pointers to const data.

Actually it doesn't matter that `struct sbus_interface` contains only pointers to const data.

What I want to say: we do not cleanup explicitly content of `tmp` variable so one might reasonably expect that `dbus = tmp;` results in a valid variable `dbus` in this example. But content of `tmp` is destroyed **implicitly** leading to invalid `dbus` that seems to be little unexpected.

Still those are nitpicks. But good if somebody else will take a look.
"""

See the full comment at https://github.com/SSSD/sssd/pull/740#issuecomment-460326525

[sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE

alexey-tikhonov commented:
"""
> *) there is an error in implementations of `sbus_method_copy/sbus_signal_copy/sbus_property_copy`:
> memcpy(copy, input, sizeof(struct sbus_property) * count + 1);
>
> should be
> memcpy(copy, input, sizeof(struct sbus_property) * (count + 1));

PR: https://github.com/SSSD/sssd/pull/741

"""

See the full comment at https://github.com/SSSD/sssd/pull/740#issuecomment-460355454

[sssd PR#740][synchronized] sbus: avoid using invalid stack point in SBUS_INTERFACE

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/740
Author: pbrezina
 Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/740/head:pr740
git checkout pr740

[sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE

pbrezina commented:
"""
Thank you Alexey, I pushed a new commit that improves the documentation and hopefully makes clear the intended usage on documentation level.

There is currently only one use case for this (to define an interface and register it with the message bus) and the reason I did it this way originally and want to keep it is to make this use case as simple as possible while having compile time check of function and private data types. If there ever will be another use case that requires more flexibility, we can deal with it then perhaps by making `sbus_interface_copy` public. But there is no need for it now and likely never will be.
"""

See the full comment at https://github.com/SSSD/sssd/pull/740#issuecomment-460573150

[sssd PR#740][+Accepted] sbus: avoid using invalid stack point in SBUS_INTERFACE

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE

Label: +Accepted

[sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE

jhrozek commented:
"""
* master:
 * e185b039468ec27bbc905c61c57dffc5496af521
 * 194438830cdd729e317c1e1baf93da7201dfd39b

"""

See the full comment at https://github.com/SSSD/sssd/pull/740#issuecomment-460801777

[sssd PR#740][closed] sbus: avoid using invalid stack point in SBUS_INTERFACE

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/740
Author: pbrezina
 Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/740/head:pr740
git checkout pr740

[sssd PR#740][+Pushed] sbus: avoid using invalid stack point in SBUS_INTERFACE

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE

Label: +Pushed