rootless: fix return value handling #16188
Conversation
[NO NEW TESTS NEEDED] Fixes: containers#15927 Signed-off-by: Erik Sjölund <[email protected]>
eriksjolund
force-pushed
the
fix_issue_15927
branch
from
34a3b5d to
cb2631b
Compare
|
|
||
| close (p[1]); | ||
| /* Block until we write the pid file. */ | ||
| r = TEMP_FAILURE_RETRY (read (p[0], &b, 1)); | ||
| close (p[0]); | ||
|
|
||
| r = reexec_in_user_namespace_wait (pid, 0); | ||
| if (r != 0) | ||
| r2 = reexec_in_user_namespace_wait (pid, 0); |
There was a problem hiding this comment.
This looks very different in the previous code reexec_in_user_namespace_wait was expected to succeed if it returned a 1, no you are failing it if returns a 1?
@giuseppe PTAL
There was a problem hiding this comment.
this looks indeed like a regression that was introduced with 9925870 as it overwrites the value we care about.
|
Edit: I removed the suggestion. Too much guessing from my side |
|
In any case the current main branch version seems to have a bug here podman/pkg/rootless/rootless_linux.c Lines 517 to 520 in 8656ffa The four lines will always result in Relevant commit |
|
Thanks, then your code looks correct, but I am not sure why @giuseppe did not totally break everything. |
|
I guess @giuseppe will know how to make a proper fix. (I don't know this code so well) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eriksjolund, giuseppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/lgtm |
|
@mheon This needs to get into 4.3 release. |
github-actions
bot
added
the
locked - please file new issue/PR
Fixes: #15927
Signed-off-by: Erik Sjölund [email protected]
Does this PR introduce a user-facing change?