Skip to content

Commit

Permalink
SELINUX: get rid of response as it was redundant and
Browse files Browse the repository at this point in the history 
rely solely on exit code.

This also fixes a bug that exit code wasn't handled before.
  • Loading branch information
@alexey-tikhonov
alexey-tikhonov committed Jan 13, 2025
1 parent 51fb6df commit 50ced44
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 164 deletions.
100 changes: 17 additions & 83 deletions src/providers/ipa/ipa_selinux.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,12 +564,11 @@ struct selinux_child_state {
};

static errno_t selinux_child_create_buffer(struct selinux_child_state *state);
static errno_t selinux_fork_child(struct selinux_child_state *state);
static errno_t selinux_fork_child(struct tevent_req *req, struct selinux_child_state *state);
static void selinux_child_step(struct tevent_req *subreq);
static void selinux_child_done(struct tevent_req *subreq);
static errno_t selinux_child_parse_response(uint8_t *buf, ssize_t len,
uint32_t *_child_result);

static void selinux_child_done(int child_status,
struct tevent_signal *sige,
void *pvt);
static struct tevent_req *selinux_child_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct selinux_child_input *sci)
Expand Down Expand Up @@ -606,7 +605,7 @@ static struct tevent_req *selinux_child_send(TALLOC_CTX *mem_ctx,
goto immediately;
}

ret = selinux_fork_child(state);
ret = selinux_fork_child(req, state);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "Failed to fork the child\n");
goto immediately;
Expand Down Expand Up @@ -671,10 +670,9 @@ static errno_t selinux_child_create_buffer(struct selinux_child_state *state)
return EOK;
}

static errno_t selinux_fork_child(struct selinux_child_state *state)
static errno_t selinux_fork_child(struct tevent_req *req, struct selinux_child_state *state)
{
int pipefd_to_child[2];
int pipefd_from_child[2];
pid_t pid;
errno_t ret;
const char **extra_args;
Expand All @@ -693,14 +691,6 @@ static errno_t selinux_fork_child(struct selinux_child_state *state)

extra_args[c] = NULL;

ret = pipe(pipefd_from_child);
if (ret == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
"pipe (from) failed [%d][%s].\n", errno, sss_strerror(errno));
return ret;
}

ret = pipe(pipefd_to_child);
if (ret == -1) {
ret = errno;
Expand All @@ -712,21 +702,19 @@ static errno_t selinux_fork_child(struct selinux_child_state *state)
pid = fork();

if (pid == 0) { /* child */
exec_child_ex(state, pipefd_to_child, pipefd_from_child,
exec_child_ex(state, pipefd_to_child, NULL,
SELINUX_CHILD, SELINUX_CHILD_LOG_FILE, extra_args,

Check warning on line 706 in src/providers/ipa/ipa_selinux.c

View workflow job for this annotation

GitHub Actions / cppcheck 

syntax error
false, STDIN_FILENO, STDOUT_FILENO);
DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec selinux_child: [%d][%s].\n",
ret, sss_strerror(ret));
return ret;
} else if (pid > 0) { /* parent */
state->io->read_from_child_fd = pipefd_from_child[0];
close(pipefd_from_child[1]);
state->io->read_from_child_fd = -1;
state->io->write_to_child_fd = pipefd_to_child[1];
close(pipefd_to_child[0]);
sss_fd_nonblocking(state->io->read_from_child_fd);
sss_fd_nonblocking(state->io->write_to_child_fd);

ret = child_handler_setup(state->ev, pid, NULL, NULL, NULL);
ret = child_handler_setup(state->ev, pid, selinux_child_done, req, NULL);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Could not set up child signal handler\n");
Expand All @@ -746,11 +734,8 @@ static void selinux_child_step(struct tevent_req *subreq)
{
struct tevent_req *req;
errno_t ret;
struct selinux_child_state *state;


req = tevent_req_callback_data(subreq, struct tevent_req);
state = tevent_req_data(req, struct selinux_child_state);

ret = write_pipe_recv(subreq);
talloc_zfree(subreq);
Expand All @@ -759,70 +744,19 @@ static void selinux_child_step(struct tevent_req *subreq)
return;
}

close(state->io->write_to_child_fd);
state->io->write_to_child_fd = -1;

subreq = read_pipe_send(state, state->ev, state->io->read_from_child_fd);
if (subreq == NULL) {
tevent_req_error(req, ENOMEM);
return;
}
tevent_req_set_callback(subreq, selinux_child_done, req);
/* selinux_child_done() to be called by sig handler */
}

static void selinux_child_done(struct tevent_req *subreq)
static void selinux_child_done(int child_status,
struct tevent_signal *,
void *pvt)
{
struct tevent_req *req;
struct selinux_child_state *state;
uint32_t child_result;
errno_t ret;
ssize_t len;
uint8_t *buf;

req = tevent_req_callback_data(subreq, struct tevent_req);
state = tevent_req_data(req, struct selinux_child_state);

ret = read_pipe_recv(subreq, state, &buf, &len);
talloc_zfree(subreq);
if (ret != EOK) {
tevent_req_error(req, ret);
return;
}

close(state->io->read_from_child_fd);
state->io->read_from_child_fd = -1;

ret = selinux_child_parse_response(buf, len, &child_result);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"selinux_child_parse_response failed: [%d][%s]\n",
ret, strerror(ret));
tevent_req_error(req, ret);
return;
} else if (child_result != 0){
DEBUG(SSSDBG_CRIT_FAILURE,
"Error in selinux_child: [%d][%s]\n",
child_result, strerror(child_result));
tevent_req_error(req, ERR_SELINUX_CONTEXT);
struct tevent_req *req = (struct tevent_req *)pvt;
if (WIFEXITED(child_status) && (WEXITSTATUS(child_status) == 0)) {
tevent_req_done(req);
return;
}

tevent_req_done(req);
return;
}

static errno_t selinux_child_parse_response(uint8_t *buf,
ssize_t len,
uint32_t *_child_result)
{
size_t p = 0;
uint32_t child_result;

/* semanage retval */
SAFEALIGN_COPY_UINT32_CHECK(&child_result, buf + p, len, &p);

*_child_result = child_result;
return EOK;
tevent_req_error(req, EFAULT);
}

static errno_t selinux_child_recv(struct tevent_req *req)
Expand Down
74 changes: 0 additions & 74 deletions src/providers/ipa/selinux_child.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,54 +110,6 @@ static errno_t unpack_buffer(uint8_t *buf,
return EOK;
}

static errno_t pack_buffer(struct response *r, int result)
{
size_t p = 0;

/* A buffer with the following structure must be created:
* uint32_t status of the request (required)
*/
r->size = sizeof(uint32_t);

r->buf = talloc_array(r, uint8_t, r->size);
if(r->buf == NULL) {
return ENOMEM;
}

DEBUG(SSSDBG_TRACE_FUNC, "result [%d]\n", result);

/* result */
SAFEALIGN_SET_UINT32(&r->buf[p], result, &p);

return EOK;
}

static errno_t prepare_response(TALLOC_CTX *mem_ctx,
int result,
struct response **rsp)
{
int ret;
struct response *r = NULL;

r = talloc_zero(mem_ctx, struct response);
if (r == NULL) {
return ENOMEM;
}

r->buf = NULL;
r->size = 0;

ret = pack_buffer(r, result);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "pack_buffer failed\n");
return ret;
}

*rsp = r;
DEBUG(SSSDBG_TRACE_ALL, "r->size: %zu\n", r->size);
return EOK;
}

static int sc_set_seuser(const char *login_name, const char *seuser_name,
const char *mls)
{
Expand Down Expand Up @@ -230,9 +182,7 @@ int main(int argc, const char *argv[])
uint8_t *buf = NULL;
ssize_t len = 0;
struct input_buffer *ibuf = NULL;
struct response *resp = NULL;
struct passwd *passwd = NULL;
ssize_t written;
bool needs_update;
const char *username;
const char *opt_logger = NULL;
Expand Down Expand Up @@ -412,30 +362,6 @@ int main(int argc, const char *argv[])
setresgid(sgid, sgid, sgid);
}

sss_log_process_caps("Sending response");

ret = prepare_response(main_ctx, EOK, &resp);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to prepare response buffer.\n");
goto fail;
}

errno = 0;

written = sss_atomic_write_s(STDOUT_FILENO, resp->buf, resp->size);
if (written == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "write failed [%d][%s].\n", ret,
strerror(ret));
goto fail;
}

if (written != resp->size) {
DEBUG(SSSDBG_CRIT_FAILURE, "Expected to write %zu bytes, wrote %zu\n",
resp->size, written);
goto fail;
}

DEBUG(SSSDBG_TRACE_FUNC, "selinux_child completed successfully\n");
close(STDOUT_FILENO);
talloc_free(main_ctx);
Expand Down
17 changes: 10 additions & 7 deletions src/util/child_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -896,13 +896,16 @@ void exec_child_ex(TALLOC_CTX *mem_ctx,
exit(EXIT_FAILURE);
}

close(pipefd_from_child[0]);
ret = dup2(pipefd_from_child[1], child_out_fd);
if (ret == -1) {
err = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
"dup2 failed [%d][%s].\n", err, strerror(err));
exit(EXIT_FAILURE);
/* some helpers, like 'selinux_child', do not write a response */
if (pipefd_from_child != NULL) {
close(pipefd_from_child[0]);
ret = dup2(pipefd_from_child[1], child_out_fd);
if (ret == -1) {
err = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
"dup2 failed [%d][%s].\n", err, strerror(err));
exit(EXIT_FAILURE);
}
}

ret = prepare_child_argv(mem_ctx, debug_fd,
Expand Down

0 comments on commit 50ced44

Please sign in to comment.