Re: [PATCH] libselinux: preserve errno in selinux_log()

[James Carter <jwcart2@gmail.com>]

On Thu, May 12, 2022 at 1:58 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, May 12, 2022 at 12:02 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > selinux_log() is used in many error branches, where the caller might
> > expect errno to bet set, e.g. label_file.c::lookup_all():
> >
> >     if (match_count) {
> >                 *match_count = 0;
> >                 result = calloc(data->nspec, sizeof(struct spec*));
> >         } else {
> >                 result = calloc(1, sizeof(struct spec*));
> >         }
> >         if (!result) {
> >                 selinux_log(SELINUX_ERROR, "Failed to allocate %zu bytes of data\n",
> >                             data->nspec * sizeof(struct spec*));
> >                 goto finish;
> >         }
> >
> > Preserve errno in the macro wrapper itself, also preventing accidental
> > errno modifications in client specified SELINUX_CB_LOG callbacks.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>

Merged.
Thanks,
Jim

> > ---
> >  libselinux/src/callbacks.h              | 3 +++
> >  libselinux/src/label_backends_android.c | 2 --
> >  libselinux/src/label_file.h             | 2 --
> >  libselinux/src/selinux_restorecon.c     | 6 +-----
> >  4 files changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/libselinux/src/callbacks.h b/libselinux/src/callbacks.h
> > index f4dab157..5a4d0f8a 100644
> > --- a/libselinux/src/callbacks.h
> > +++ b/libselinux/src/callbacks.h
> > @@ -5,6 +5,7 @@
> >  #ifndef _SELINUX_CALLBACKS_H_
> >  #define _SELINUX_CALLBACKS_H_
> >
> > +#include <errno.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > @@ -32,9 +33,11 @@ extern int
> >  extern pthread_mutex_t log_mutex;
> >
> >  #define selinux_log(type, ...) do { \
> > +       int saved_errno__ = errno; \
> >         __pthread_mutex_lock(&log_mutex); \
> >         selinux_log_direct(type, __VA_ARGS__); \
> >         __pthread_mutex_unlock(&log_mutex); \
> > +       errno = saved_errno__; \
> >  } while(0)
> >
> >  #endif                         /* _SELINUX_CALLBACKS_H_ */
> > diff --git a/libselinux/src/label_backends_android.c b/libselinux/src/label_backends_android.c
> > index 66d4df2d..c2d78360 100644
> > --- a/libselinux/src/label_backends_android.c
> > +++ b/libselinux/src/label_backends_android.c
> > @@ -93,7 +93,6 @@ static int process_line(struct selabel_handle *rec,
> >
> >         items = read_spec_entries(line_buf, &errbuf, 2, &prop, &context);
> >         if (items < 0) {
> > -               items = errno;
> >                 if (errbuf) {
> >                         selinux_log(SELINUX_ERROR,
> >                                     "%s:  line %u error due to: %s\n", path,
> > @@ -103,7 +102,6 @@ static int process_line(struct selabel_handle *rec,
> >                                     "%s:  line %u error due to: %m\n", path,
> >                                     lineno);
> >                 }
> > -               errno = items;
> >                 return -1;
> >         }
> >
> > diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
> > index b453e13f..190bc175 100644
> > --- a/libselinux/src/label_file.h
> > +++ b/libselinux/src/label_file.h
> > @@ -444,7 +444,6 @@ static inline int process_line(struct selabel_handle *rec,
> >
> >         items = read_spec_entries(line_buf, &errbuf, 3, &regex, &type, &context);
> >         if (items < 0) {
> > -               rc = errno;
> >                 if (errbuf) {
> >                         selinux_log(SELINUX_ERROR,
> >                                     "%s:  line %u error due to: %s\n", path,
> > @@ -454,7 +453,6 @@ static inline int process_line(struct selabel_handle *rec,
> >                                     "%s:  line %u error due to: %m\n", path,
> >                                     lineno);
> >                 }
> > -               errno = rc;
> >                 return -1;
> >         }
> >
> > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> > index e6192912..ba7b3692 100644
> > --- a/libselinux/src/selinux_restorecon.c
> > +++ b/libselinux/src/selinux_restorecon.c
> > @@ -1032,7 +1032,7 @@ static int selinux_restorecon_common(const char *pathname_orig,
> >         struct stat sb;
> >         char *pathname = NULL, *pathdnamer = NULL, *pathdname, *pathbname;
> >         char *paths[2] = { NULL, NULL };
> > -       int fts_flags, error, sverrno;
> > +       int fts_flags, error;
> >         struct dir_hash_node *current = NULL;
> >
> >         if (state.flags.verbose && state.flags.progress)
> > @@ -1286,18 +1286,14 @@ cleanup:
> >         return error;
> >
> >  oom:
> > -       sverrno = errno;
> >         selinux_log(SELINUX_ERROR, "%s:  Out of memory\n", __func__);
> > -       errno = sverrno;
> >         error = -1;
> >         goto cleanup;
> >
> >  realpatherr:
> > -       sverrno = errno;
> >         selinux_log(SELINUX_ERROR,
> >                     "SELinux: Could not get canonical path for %s restorecon: %m.\n",
> >                     pathname_orig);
> > -       errno = sverrno;
> >         error = -1;
> >         goto cleanup;
> >
> > --
> > 2.36.1
> >