sd-bus: deal with cookie overruns #11818
Conversation
Apparently this happens IRL. Let's carefully deal with issues like this: when we overrun, let's not go back to zero but instead leave the highest cookie bit set. We use that as indication that we are in "overrun territory", and then are particularly careful with checking cookies, i.e. that they haven't been used for still outstanding replies yet. This should retain the quick cookie generation behaviour we used to have, but permits dealing with overruns. Replaces: systemd#11804 Fixes: systemd#11809
poettering
force-pushed
the
bus-cookie-overrun
branch
from
98516a7
to
3ff71f0
Compare
| goto good; | ||
|
|
||
| new_cookie = cookie_inc(new_cookie); | ||
| } |
There was a problem hiding this comment.
Hmm, so if get to the for loop, new_cookie must be COOKIE_CYCLED, and it is never increased again. So this is an "infinite" loop, in the sense that we'll always do full 2**31 iterations.
Can we please have a unittest for this?
There was a problem hiding this comment.
no, it goes like this:
1 → 2 → 3 → … → CC-2 → CC-1 → CC → CC+1 → CC+2 → … → UM-2 → UM-1 → UM
↑ ↓
← ← ← ← ← ← ← ← ← ← ← ← ← ← ← ← ← ← ←
Were CC is COOKIE_CYCLED is 2^31 and UM is UINT32_MAX-1, i.e. 2^32-1
There was a problem hiding this comment.
Indeed. I misunderstood the code.
So this looks OK, and I think we can merge it. A test would still be very welcome.
Maybe also add your comment above as a comment in the code.
keszybz
added
the
reviewed/needs-rework 🔨
| goto good; | ||
|
|
||
| new_cookie = cookie_inc(new_cookie); | ||
| } |
There was a problem hiding this comment.
Indeed. I misunderstood the code.
So this looks OK, and I think we can merge it. A test would still be very welcome.
Maybe also add your comment above as a comment in the code.
keszybz
removed
the
reviewed/needs-rework 🔨
Apparently this happens IRL. Let's carefully deal with issues like this:
when we overrun, let's not go back to zero but instead leave the highest
cookie bit set. We use that as indication that we are in "overrun
territory", and then are particularly careful with checking cookies,
i.e. that they haven't been used for still outstanding replies yet. This
should retain the quick cookie generation behaviour we used to have, but
permits dealing with overruns.
Replaces: #11804