Hello, Sorry for not answering sooner. I was busy in the past few months, and it seems the other libselinux maintainers too. I had some difficulty understanding what your patch was about, as I missed its context (from https://lore.kernel.org/selinux/20220121084012.GS7643@xxxxxxxx/ ) and the commit message did not explain what the issue actually was. While trying to reverse-engineer it, I found (again) the bug which is fixed. Instead of adding "pid == 0" to some functions, I decided to rework/simplify libselinux/src/procattr.c to fix this bug. The result of this work was sent on the mailing list: https://patchwork.kernel.org/project/selinux/patch/20220529180111.408899-1-nicolas.iooss@xxxxxxx/ . I personally prefer this simplification, but if anyone thinks this makes it more difficult to maintain libselinux (for example), I am open to discussion. Best regards, Nicolas On Thu, Jan 27, 2022 at 2:07 PM Johannes Segitz <jsegitz@xxxxxxx> wrote: > > The procattr cache doesn't work properly in all cases. This fixes the issue at > the cost of not using the cache as soon as a pid is specified. > > In most use cases this will never occur, but it's still a small security issue, > since this incorrect information might lead to incorrect access decisions. > > Signed-off-by: Johannes Segitz <jsegitz@xxxxxxx> > --- > libselinux/src/procattr.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c > index 142fbf3a..4ca8337a 100644 > --- a/libselinux/src/procattr.c > +++ b/libselinux/src/procattr.c > @@ -148,7 +148,7 @@ static int getprocattrcon_raw(char ** context, > return -1; > } > > - if (prev_context && prev_context != UNSET) { > + if (pid == 0 && prev_context && prev_context != UNSET) { > *context = strdup(prev_context); > if (!(*context)) { > return -1; > @@ -242,9 +242,9 @@ static int setprocattrcon_raw(const char * context, > return -1; > } > > - if (!context && !*prev_context) > + if (pid == 0 && !context && !*prev_context) > return 0; > - if (context && *prev_context && *prev_context != UNSET > + if (pid == 0 && context && *prev_context && *prev_context != UNSET > && !strcmp(context, *prev_context)) > return 0; > > @@ -272,9 +272,11 @@ out: > free(context2); > return -1; > } else { > - if (*prev_context != UNSET) > - free(*prev_context); > - *prev_context = context2; > + if (pid == 0) { > + if (*prev_context != UNSET) > + free(*prev_context); > + *prev_context = context2; > + } > return 0; > } > } > -- > 2.31.1 >
