5:07 a.m.
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
11 a.m.
New subject: [sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE
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
12:20 p.m.
New subject: [sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE
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
3:37 a.m.
New subject: [sssd PR#740][synchronized] sbus: avoid using invalid stack point in SBUS_INTERFACE
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
3:43 a.m.
New subject: [sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE
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
9:12 a.m.
New subject: [sssd PR#740][+Accepted] sbus: avoid using invalid stack point in SBUS_INTERFACE
URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE
Label: +Accepted
3 p.m.
New subject: [sssd PR#740][comment] sbus: avoid using invalid stack point in SBUS_INTERFACE
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
3:01 p.m.
New subject: [sssd PR#740][closed] sbus: avoid using invalid stack point in SBUS_INTERFACE
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
3:01 p.m.
New subject: [sssd PR#740][+Pushed] sbus: avoid using invalid stack point in SBUS_INTERFACE
URL: https://github.com/SSSD/sssd/pull/740
Title: #740: sbus: avoid using invalid stack point in SBUS_INTERFACE
Label: +Pushed
1959
days inactive
1960
days old
sssd-devel@lists.fedorahosted.org
8 comments
3 participants
participants (3)
-
alexey-tikhonov -
jhrozek
-
pbrezina