diff mbox series

[1/1] libselinux: do not return the cached prev_current value when using getpidcon()

Message ID 20220529180111.408899-1-nicolas.iooss@m4x.org (mailing list archive)
State Accepted
Commit c8ba7968b3ab
Headers show
Series [1/1] libselinux: do not return the cached prev_current value when using getpidcon() | expand

Commit Message

Nicolas Iooss May 29, 2022, 6:01 p.m. UTC
libselinux implements a cache mechanism for get*con() functions, such
that when a thread calls setcon(...) then getcon(...), the context is
directly returned. Unfortunately, getpidcon(pid, &context) uses the same
cached variable, so when a program uses setcon("something"), all later
calls to getpidcon(pid, ...) returns "something". This is a bug.

Here is a program which illustrates this bug:

    #include <stdio.h>
    #include <selinux/selinux.h>

    int main() {
        char *context = "";
        if (getpidcon(1, &context) < 0) {
            perror("getpidcon(1)");
        }
        printf("getpidcon(1) = %s\n", context);

        if (getcon(&context) < 0) {
            perror("getcon()");
        }
        printf("getcon() = %s\n", context);
        if (setcon(context) < 0) {
            perror("setcon()");
        }
        if (getpidcon(1, &context) < 0) {
            perror("getpidcon(1)");
        }
        printf("getpidcon(1) = %s\n", context);

        return 0;
    }

On an Arch Linux system using unconfined user, this program displays:

    getpidcon(1) = system_u:system_r:init_t
    getcon() = unconfined_u:unconfined_r:unconfined_t
    getpidcon(1) = unconfined_u:unconfined_r:unconfined_t

With this commit, this program displays:

    getpidcon(1) = system_u:system_r:init_t
    getcon() = unconfined_u:unconfined_r:unconfined_t
    getpidcon(1) = system_u:system_r:init_t

This bug was present in the first commit of
https://github.com/SELinuxProject/selinux git history. It was reported
in https://lore.kernel.org/selinux/20220121084012.GS7643@suse.com/ and a
patch to fix it was sent in
https://patchwork.kernel.org/project/selinux/patch/20220127130741.31940-1-jsegitz@suse.de/
without a clear explanation. This patch added pid checks, which made
sense but were difficult to read. Instead, it is possible to change the
way the functions are called so that they directly know which cache
variable to use.

Moreover, as the code is not clear at all (I spent too much time trying
to understand what the switch did and what the thread-local variable
contained), this commit also reworks libselinux/src/procattr.c to:
- not use hard-to-understand switch/case constructions on strings (they
  are replaced by a new argument filled by macros)
- remove getpidattr_def macro (it was only used once, for pidcon, and
  the code is clearer with one less macro)
- remove the pid parameter of setprocattrcon() and setprocattrcon_raw()
  (it is always zero)

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Cc: Johannes Segitz <jsegitz@suse.de>
---
 libselinux/src/procattr.c | 147 +++++++++++++-------------------------
 1 file changed, 50 insertions(+), 97 deletions(-)

Comments

James Carter June 2, 2022, 1:56 p.m. UTC | #1
On Mon, May 30, 2022 at 3:09 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> libselinux implements a cache mechanism for get*con() functions, such
> that when a thread calls setcon(...) then getcon(...), the context is
> directly returned. Unfortunately, getpidcon(pid, &context) uses the same
> cached variable, so when a program uses setcon("something"), all later
> calls to getpidcon(pid, ...) returns "something". This is a bug.
>
> Here is a program which illustrates this bug:
>
>     #include <stdio.h>
>     #include <selinux/selinux.h>
>
>     int main() {
>         char *context = "";
>         if (getpidcon(1, &context) < 0) {
>             perror("getpidcon(1)");
>         }
>         printf("getpidcon(1) = %s\n", context);
>
>         if (getcon(&context) < 0) {
>             perror("getcon()");
>         }
>         printf("getcon() = %s\n", context);
>         if (setcon(context) < 0) {
>             perror("setcon()");
>         }
>         if (getpidcon(1, &context) < 0) {
>             perror("getpidcon(1)");
>         }
>         printf("getpidcon(1) = %s\n", context);
>
>         return 0;
>     }
>
> On an Arch Linux system using unconfined user, this program displays:
>
>     getpidcon(1) = system_u:system_r:init_t
>     getcon() = unconfined_u:unconfined_r:unconfined_t
>     getpidcon(1) = unconfined_u:unconfined_r:unconfined_t
>
> With this commit, this program displays:
>
>     getpidcon(1) = system_u:system_r:init_t
>     getcon() = unconfined_u:unconfined_r:unconfined_t
>     getpidcon(1) = system_u:system_r:init_t
>
> This bug was present in the first commit of
> https://github.com/SELinuxProject/selinux git history. It was reported
> in https://lore.kernel.org/selinux/20220121084012.GS7643@suse.com/ and a
> patch to fix it was sent in
> https://patchwork.kernel.org/project/selinux/patch/20220127130741.31940-1-jsegitz@suse.de/
> without a clear explanation. This patch added pid checks, which made
> sense but were difficult to read. Instead, it is possible to change the
> way the functions are called so that they directly know which cache
> variable to use.
>
> Moreover, as the code is not clear at all (I spent too much time trying
> to understand what the switch did and what the thread-local variable
> contained), this commit also reworks libselinux/src/procattr.c to:
> - not use hard-to-understand switch/case constructions on strings (they
>   are replaced by a new argument filled by macros)
> - remove getpidattr_def macro (it was only used once, for pidcon, and
>   the code is clearer with one less macro)
> - remove the pid parameter of setprocattrcon() and setprocattrcon_raw()
>   (it is always zero)
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> Cc: Johannes Segitz <jsegitz@suse.de>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  libselinux/src/procattr.c | 147 +++++++++++++-------------------------
>  1 file changed, 50 insertions(+), 97 deletions(-)
>
> diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
> index 142fbf3a80e0..6f4cfb82479d 100644
> --- a/libselinux/src/procattr.c
> +++ b/libselinux/src/procattr.c
> @@ -11,11 +11,14 @@
>
>  #define UNSET (char *) -1
>
> +/* Cached values so that when a thread calls set*con() then gen*con(), the value
> + * which was set is directly returned.
> + */
>  static __thread char *prev_current = UNSET;
> -static __thread char * prev_exec = UNSET;
> -static __thread char * prev_fscreate = UNSET;
> -static __thread char * prev_keycreate = UNSET;
> -static __thread char * prev_sockcreate = UNSET;
> +static __thread char *prev_exec = UNSET;
> +static __thread char *prev_fscreate = UNSET;
> +static __thread char *prev_keycreate = UNSET;
> +static __thread char *prev_sockcreate = UNSET;
>
>  static pthread_once_t once = PTHREAD_ONCE_INIT;
>  static pthread_key_t destructor_key;
> @@ -111,43 +114,18 @@ out:
>         return fd;
>  }
>
> -static int getprocattrcon_raw(char ** context,
> -                             pid_t pid, const char *attr)
> +static int getprocattrcon_raw(char **context, pid_t pid, const char *attr,
> +                             const char *prev_context)
>  {
>         char *buf;
>         size_t size;
>         int fd;
>         ssize_t ret;
>         int errno_hold;
> -       char * prev_context;
>
>         __selinux_once(once, init_procattr);
>         init_thread_destructor();
>
> -       switch (attr[0]) {
> -               case 'c':
> -                       prev_context = prev_current;
> -                       break;
> -               case 'e':
> -                       prev_context = prev_exec;
> -                       break;
> -               case 'f':
> -                       prev_context = prev_fscreate;
> -                       break;
> -               case 'k':
> -                       prev_context = prev_keycreate;
> -                       break;
> -               case 's':
> -                       prev_context = prev_sockcreate;
> -                       break;
> -               case 'p':
> -                       prev_context = NULL;
> -                       break;
> -               default:
> -                       errno = ENOENT;
> -                       return -1;
> -       }
> -
>         if (prev_context && prev_context != UNSET) {
>                 *context = strdup(prev_context);
>                 if (!(*context)) {
> @@ -194,13 +172,13 @@ static int getprocattrcon_raw(char ** context,
>         return ret;
>  }
>
> -static int getprocattrcon(char ** context,
> -                         pid_t pid, const char *attr)
> +static int getprocattrcon(char **context, pid_t pid, const char *attr,
> +                         const char *prev_context)
>  {
>         int ret;
>         char * rcontext;
>
> -       ret = getprocattrcon_raw(&rcontext, pid, attr);
> +       ret = getprocattrcon_raw(&rcontext, pid, attr, prev_context);
>
>         if (!ret) {
>                 ret = selinux_raw_to_trans_context(rcontext, context);
> @@ -210,45 +188,24 @@ static int getprocattrcon(char ** context,
>         return ret;
>  }
>
> -static int setprocattrcon_raw(const char * context,
> -                             pid_t pid, const char *attr)
> +static int setprocattrcon_raw(const char *context, const char *attr,
> +                             char **prev_context)
>  {
>         int fd;
>         ssize_t ret;
>         int errno_hold;
> -       char **prev_context, *context2 = NULL;
> +       char *context2 = NULL;
>
>         __selinux_once(once, init_procattr);
>         init_thread_destructor();
>
> -       switch (attr[0]) {
> -               case 'c':
> -                       prev_context = &prev_current;
> -                       break;
> -               case 'e':
> -                       prev_context = &prev_exec;
> -                       break;
> -               case 'f':
> -                       prev_context = &prev_fscreate;
> -                       break;
> -               case 'k':
> -                       prev_context = &prev_keycreate;
> -                       break;
> -               case 's':
> -                       prev_context = &prev_sockcreate;
> -                       break;
> -               default:
> -                       errno = ENOENT;
> -                       return -1;
> -       }
> -
>         if (!context && !*prev_context)
>                 return 0;
>         if (context && *prev_context && *prev_context != UNSET
>             && !strcmp(context, *prev_context))
>                 return 0;
>
> -       fd = openattr(pid, attr, O_RDWR | O_CLOEXEC);
> +       fd = openattr(0, attr, O_RDWR | O_CLOEXEC);
>         if (fd < 0)
>                 return -1;
>         if (context) {
> @@ -279,8 +236,8 @@ out:
>         }
>  }
>
> -static int setprocattrcon(const char * context,
> -                         pid_t pid, const char *attr)
> +static int setprocattrcon(const char *context, const char *attr,
> +                         char **prev_context)
>  {
>         int ret;
>         char * rcontext;
> @@ -288,62 +245,58 @@ static int setprocattrcon(const char * context,
>         if (selinux_trans_to_raw_context(context, &rcontext))
>                 return -1;
>
> -       ret = setprocattrcon_raw(rcontext, pid, attr);
> +       ret = setprocattrcon_raw(rcontext, attr, prev_context);
>
>         freecon(rcontext);
>
>         return ret;
>  }
>
> -#define getselfattr_def(fn, attr) \
> +#define getselfattr_def(fn, attr, prev_context) \
>         int get##fn##_raw(char **c) \
>         { \
> -               return getprocattrcon_raw(c, 0, #attr); \
> +               return getprocattrcon_raw(c, 0, attr, prev_context); \
>         } \
>         int get##fn(char **c) \
>         { \
> -               return getprocattrcon(c, 0, #attr); \
> +               return getprocattrcon(c, 0, attr, prev_context); \
>         }
>
> -#define setselfattr_def(fn, attr) \
> +#define setselfattr_def(fn, attr, prev_context) \
>         int set##fn##_raw(const char * c) \
>         { \
> -               return setprocattrcon_raw(c, 0, #attr); \
> +               return setprocattrcon_raw(c, attr, &prev_context); \
>         } \
>         int set##fn(const char * c) \
>         { \
> -               return setprocattrcon(c, 0, #attr); \
> +               return setprocattrcon(c, attr, &prev_context); \
>         }
>
> -#define all_selfattr_def(fn, attr) \
> -       getselfattr_def(fn, attr)        \
> -       setselfattr_def(fn, attr)
> +#define all_selfattr_def(fn, attr, prev_context) \
> +       getselfattr_def(fn, attr, prev_context)  \
> +       setselfattr_def(fn, attr, prev_context)
>
> -#define getpidattr_def(fn, attr) \
> -       int get##fn##_raw(pid_t pid, char **c)  \
> -       { \
> -               if (pid <= 0) { \
> -                       errno = EINVAL; \
> -                       return -1; \
> -               } else { \
> -                       return getprocattrcon_raw(c, pid, #attr); \
> -               } \
> -       } \
> -       int get##fn(pid_t pid, char **c)        \
> -       { \
> -               if (pid <= 0) { \
> -                       errno = EINVAL; \
> -                       return -1; \
> -               } else { \
> -                       return getprocattrcon(c, pid, #attr); \
> -               } \
> -       }
> +all_selfattr_def(con, "current", prev_current)
> +    getselfattr_def(prevcon, "prev", NULL)
> +    all_selfattr_def(execcon, "exec", prev_exec)
> +    all_selfattr_def(fscreatecon, "fscreate", prev_fscreate)
> +    all_selfattr_def(sockcreatecon, "sockcreate", prev_sockcreate)
> +    all_selfattr_def(keycreatecon, "keycreate", prev_keycreate)
>
> -all_selfattr_def(con, current)
> -    getpidattr_def(pidcon, current)
> -    getselfattr_def(prevcon, prev)
> -    all_selfattr_def(execcon, exec)
> -    all_selfattr_def(fscreatecon, fscreate)
> -    all_selfattr_def(sockcreatecon, sockcreate)
> -    all_selfattr_def(keycreatecon, keycreate)
> +int getpidcon_raw(pid_t pid, char **c)
> +{
> +       if (pid <= 0) {
> +               errno = EINVAL;
> +               return -1;
> +       }
> +       return getprocattrcon_raw(c, pid, "current", NULL);
> +}
>
> +int getpidcon(pid_t pid, char **c)
> +{
> +       if (pid <= 0) {
> +               errno = EINVAL;
> +               return -1;
> +       }
> +       return getprocattrcon(c, pid, "current", NULL);
> +}
> --
> 2.36.1
>
James Carter June 6, 2022, 8:43 p.m. UTC | #2
On Thu, Jun 2, 2022 at 9:56 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Mon, May 30, 2022 at 3:09 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > libselinux implements a cache mechanism for get*con() functions, such
> > that when a thread calls setcon(...) then getcon(...), the context is
> > directly returned. Unfortunately, getpidcon(pid, &context) uses the same
> > cached variable, so when a program uses setcon("something"), all later
> > calls to getpidcon(pid, ...) returns "something". This is a bug.
> >
> > Here is a program which illustrates this bug:
> >
> >     #include <stdio.h>
> >     #include <selinux/selinux.h>
> >
> >     int main() {
> >         char *context = "";
> >         if (getpidcon(1, &context) < 0) {
> >             perror("getpidcon(1)");
> >         }
> >         printf("getpidcon(1) = %s\n", context);
> >
> >         if (getcon(&context) < 0) {
> >             perror("getcon()");
> >         }
> >         printf("getcon() = %s\n", context);
> >         if (setcon(context) < 0) {
> >             perror("setcon()");
> >         }
> >         if (getpidcon(1, &context) < 0) {
> >             perror("getpidcon(1)");
> >         }
> >         printf("getpidcon(1) = %s\n", context);
> >
> >         return 0;
> >     }
> >
> > On an Arch Linux system using unconfined user, this program displays:
> >
> >     getpidcon(1) = system_u:system_r:init_t
> >     getcon() = unconfined_u:unconfined_r:unconfined_t
> >     getpidcon(1) = unconfined_u:unconfined_r:unconfined_t
> >
> > With this commit, this program displays:
> >
> >     getpidcon(1) = system_u:system_r:init_t
> >     getcon() = unconfined_u:unconfined_r:unconfined_t
> >     getpidcon(1) = system_u:system_r:init_t
> >
> > This bug was present in the first commit of
> > https://github.com/SELinuxProject/selinux git history. It was reported
> > in https://lore.kernel.org/selinux/20220121084012.GS7643@suse.com/ and a
> > patch to fix it was sent in
> > https://patchwork.kernel.org/project/selinux/patch/20220127130741.31940-1-jsegitz@suse.de/
> > without a clear explanation. This patch added pid checks, which made
> > sense but were difficult to read. Instead, it is possible to change the
> > way the functions are called so that they directly know which cache
> > variable to use.
> >
> > Moreover, as the code is not clear at all (I spent too much time trying
> > to understand what the switch did and what the thread-local variable
> > contained), this commit also reworks libselinux/src/procattr.c to:
> > - not use hard-to-understand switch/case constructions on strings (they
> >   are replaced by a new argument filled by macros)
> > - remove getpidattr_def macro (it was only used once, for pidcon, and
> >   the code is clearer with one less macro)
> > - remove the pid parameter of setprocattrcon() and setprocattrcon_raw()
> >   (it is always zero)
> >
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> > Cc: Johannes Segitz <jsegitz@suse.de>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>

Merged.
Thanks,
Jim

> > ---
> >  libselinux/src/procattr.c | 147 +++++++++++++-------------------------
> >  1 file changed, 50 insertions(+), 97 deletions(-)
> >
> > diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
> > index 142fbf3a80e0..6f4cfb82479d 100644
> > --- a/libselinux/src/procattr.c
> > +++ b/libselinux/src/procattr.c
> > @@ -11,11 +11,14 @@
> >
> >  #define UNSET (char *) -1
> >
> > +/* Cached values so that when a thread calls set*con() then gen*con(), the value
> > + * which was set is directly returned.
> > + */
> >  static __thread char *prev_current = UNSET;
> > -static __thread char * prev_exec = UNSET;
> > -static __thread char * prev_fscreate = UNSET;
> > -static __thread char * prev_keycreate = UNSET;
> > -static __thread char * prev_sockcreate = UNSET;
> > +static __thread char *prev_exec = UNSET;
> > +static __thread char *prev_fscreate = UNSET;
> > +static __thread char *prev_keycreate = UNSET;
> > +static __thread char *prev_sockcreate = UNSET;
> >
> >  static pthread_once_t once = PTHREAD_ONCE_INIT;
> >  static pthread_key_t destructor_key;
> > @@ -111,43 +114,18 @@ out:
> >         return fd;
> >  }
> >
> > -static int getprocattrcon_raw(char ** context,
> > -                             pid_t pid, const char *attr)
> > +static int getprocattrcon_raw(char **context, pid_t pid, const char *attr,
> > +                             const char *prev_context)
> >  {
> >         char *buf;
> >         size_t size;
> >         int fd;
> >         ssize_t ret;
> >         int errno_hold;
> > -       char * prev_context;
> >
> >         __selinux_once(once, init_procattr);
> >         init_thread_destructor();
> >
> > -       switch (attr[0]) {
> > -               case 'c':
> > -                       prev_context = prev_current;
> > -                       break;
> > -               case 'e':
> > -                       prev_context = prev_exec;
> > -                       break;
> > -               case 'f':
> > -                       prev_context = prev_fscreate;
> > -                       break;
> > -               case 'k':
> > -                       prev_context = prev_keycreate;
> > -                       break;
> > -               case 's':
> > -                       prev_context = prev_sockcreate;
> > -                       break;
> > -               case 'p':
> > -                       prev_context = NULL;
> > -                       break;
> > -               default:
> > -                       errno = ENOENT;
> > -                       return -1;
> > -       }
> > -
> >         if (prev_context && prev_context != UNSET) {
> >                 *context = strdup(prev_context);
> >                 if (!(*context)) {
> > @@ -194,13 +172,13 @@ static int getprocattrcon_raw(char ** context,
> >         return ret;
> >  }
> >
> > -static int getprocattrcon(char ** context,
> > -                         pid_t pid, const char *attr)
> > +static int getprocattrcon(char **context, pid_t pid, const char *attr,
> > +                         const char *prev_context)
> >  {
> >         int ret;
> >         char * rcontext;
> >
> > -       ret = getprocattrcon_raw(&rcontext, pid, attr);
> > +       ret = getprocattrcon_raw(&rcontext, pid, attr, prev_context);
> >
> >         if (!ret) {
> >                 ret = selinux_raw_to_trans_context(rcontext, context);
> > @@ -210,45 +188,24 @@ static int getprocattrcon(char ** context,
> >         return ret;
> >  }
> >
> > -static int setprocattrcon_raw(const char * context,
> > -                             pid_t pid, const char *attr)
> > +static int setprocattrcon_raw(const char *context, const char *attr,
> > +                             char **prev_context)
> >  {
> >         int fd;
> >         ssize_t ret;
> >         int errno_hold;
> > -       char **prev_context, *context2 = NULL;
> > +       char *context2 = NULL;
> >
> >         __selinux_once(once, init_procattr);
> >         init_thread_destructor();
> >
> > -       switch (attr[0]) {
> > -               case 'c':
> > -                       prev_context = &prev_current;
> > -                       break;
> > -               case 'e':
> > -                       prev_context = &prev_exec;
> > -                       break;
> > -               case 'f':
> > -                       prev_context = &prev_fscreate;
> > -                       break;
> > -               case 'k':
> > -                       prev_context = &prev_keycreate;
> > -                       break;
> > -               case 's':
> > -                       prev_context = &prev_sockcreate;
> > -                       break;
> > -               default:
> > -                       errno = ENOENT;
> > -                       return -1;
> > -       }
> > -
> >         if (!context && !*prev_context)
> >                 return 0;
> >         if (context && *prev_context && *prev_context != UNSET
> >             && !strcmp(context, *prev_context))
> >                 return 0;
> >
> > -       fd = openattr(pid, attr, O_RDWR | O_CLOEXEC);
> > +       fd = openattr(0, attr, O_RDWR | O_CLOEXEC);
> >         if (fd < 0)
> >                 return -1;
> >         if (context) {
> > @@ -279,8 +236,8 @@ out:
> >         }
> >  }
> >
> > -static int setprocattrcon(const char * context,
> > -                         pid_t pid, const char *attr)
> > +static int setprocattrcon(const char *context, const char *attr,
> > +                         char **prev_context)
> >  {
> >         int ret;
> >         char * rcontext;
> > @@ -288,62 +245,58 @@ static int setprocattrcon(const char * context,
> >         if (selinux_trans_to_raw_context(context, &rcontext))
> >                 return -1;
> >
> > -       ret = setprocattrcon_raw(rcontext, pid, attr);
> > +       ret = setprocattrcon_raw(rcontext, attr, prev_context);
> >
> >         freecon(rcontext);
> >
> >         return ret;
> >  }
> >
> > -#define getselfattr_def(fn, attr) \
> > +#define getselfattr_def(fn, attr, prev_context) \
> >         int get##fn##_raw(char **c) \
> >         { \
> > -               return getprocattrcon_raw(c, 0, #attr); \
> > +               return getprocattrcon_raw(c, 0, attr, prev_context); \
> >         } \
> >         int get##fn(char **c) \
> >         { \
> > -               return getprocattrcon(c, 0, #attr); \
> > +               return getprocattrcon(c, 0, attr, prev_context); \
> >         }
> >
> > -#define setselfattr_def(fn, attr) \
> > +#define setselfattr_def(fn, attr, prev_context) \
> >         int set##fn##_raw(const char * c) \
> >         { \
> > -               return setprocattrcon_raw(c, 0, #attr); \
> > +               return setprocattrcon_raw(c, attr, &prev_context); \
> >         } \
> >         int set##fn(const char * c) \
> >         { \
> > -               return setprocattrcon(c, 0, #attr); \
> > +               return setprocattrcon(c, attr, &prev_context); \
> >         }
> >
> > -#define all_selfattr_def(fn, attr) \
> > -       getselfattr_def(fn, attr)        \
> > -       setselfattr_def(fn, attr)
> > +#define all_selfattr_def(fn, attr, prev_context) \
> > +       getselfattr_def(fn, attr, prev_context)  \
> > +       setselfattr_def(fn, attr, prev_context)
> >
> > -#define getpidattr_def(fn, attr) \
> > -       int get##fn##_raw(pid_t pid, char **c)  \
> > -       { \
> > -               if (pid <= 0) { \
> > -                       errno = EINVAL; \
> > -                       return -1; \
> > -               } else { \
> > -                       return getprocattrcon_raw(c, pid, #attr); \
> > -               } \
> > -       } \
> > -       int get##fn(pid_t pid, char **c)        \
> > -       { \
> > -               if (pid <= 0) { \
> > -                       errno = EINVAL; \
> > -                       return -1; \
> > -               } else { \
> > -                       return getprocattrcon(c, pid, #attr); \
> > -               } \
> > -       }
> > +all_selfattr_def(con, "current", prev_current)
> > +    getselfattr_def(prevcon, "prev", NULL)
> > +    all_selfattr_def(execcon, "exec", prev_exec)
> > +    all_selfattr_def(fscreatecon, "fscreate", prev_fscreate)
> > +    all_selfattr_def(sockcreatecon, "sockcreate", prev_sockcreate)
> > +    all_selfattr_def(keycreatecon, "keycreate", prev_keycreate)
> >
> > -all_selfattr_def(con, current)
> > -    getpidattr_def(pidcon, current)
> > -    getselfattr_def(prevcon, prev)
> > -    all_selfattr_def(execcon, exec)
> > -    all_selfattr_def(fscreatecon, fscreate)
> > -    all_selfattr_def(sockcreatecon, sockcreate)
> > -    all_selfattr_def(keycreatecon, keycreate)
> > +int getpidcon_raw(pid_t pid, char **c)
> > +{
> > +       if (pid <= 0) {
> > +               errno = EINVAL;
> > +               return -1;
> > +       }
> > +       return getprocattrcon_raw(c, pid, "current", NULL);
> > +}
> >
> > +int getpidcon(pid_t pid, char **c)
> > +{
> > +       if (pid <= 0) {
> > +               errno = EINVAL;
> > +               return -1;
> > +       }
> > +       return getprocattrcon(c, pid, "current", NULL);
> > +}
> > --
> > 2.36.1
> >
diff mbox series

Patch

diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
index 142fbf3a80e0..6f4cfb82479d 100644
--- a/libselinux/src/procattr.c
+++ b/libselinux/src/procattr.c
@@ -11,11 +11,14 @@ 
 
 #define UNSET (char *) -1
 
+/* Cached values so that when a thread calls set*con() then gen*con(), the value
+ * which was set is directly returned.
+ */
 static __thread char *prev_current = UNSET;
-static __thread char * prev_exec = UNSET;
-static __thread char * prev_fscreate = UNSET;
-static __thread char * prev_keycreate = UNSET;
-static __thread char * prev_sockcreate = UNSET;
+static __thread char *prev_exec = UNSET;
+static __thread char *prev_fscreate = UNSET;
+static __thread char *prev_keycreate = UNSET;
+static __thread char *prev_sockcreate = UNSET;
 
 static pthread_once_t once = PTHREAD_ONCE_INIT;
 static pthread_key_t destructor_key;
@@ -111,43 +114,18 @@  out:
 	return fd;
 }
 
-static int getprocattrcon_raw(char ** context,
-			      pid_t pid, const char *attr)
+static int getprocattrcon_raw(char **context, pid_t pid, const char *attr,
+			      const char *prev_context)
 {
 	char *buf;
 	size_t size;
 	int fd;
 	ssize_t ret;
 	int errno_hold;
-	char * prev_context;
 
 	__selinux_once(once, init_procattr);
 	init_thread_destructor();
 
-	switch (attr[0]) {
-		case 'c':
-			prev_context = prev_current;
-			break;
-		case 'e':
-			prev_context = prev_exec;
-			break;
-		case 'f':
-			prev_context = prev_fscreate;
-			break;
-		case 'k':
-			prev_context = prev_keycreate;
-			break;
-		case 's':
-			prev_context = prev_sockcreate;
-			break;
-		case 'p':
-			prev_context = NULL;
-			break;
-		default:
-			errno = ENOENT;
-			return -1;
-	}
-
 	if (prev_context && prev_context != UNSET) {
 		*context = strdup(prev_context);
 		if (!(*context)) {
@@ -194,13 +172,13 @@  static int getprocattrcon_raw(char ** context,
 	return ret;
 }
 
-static int getprocattrcon(char ** context,
-			  pid_t pid, const char *attr)
+static int getprocattrcon(char **context, pid_t pid, const char *attr,
+			  const char *prev_context)
 {
 	int ret;
 	char * rcontext;
 
-	ret = getprocattrcon_raw(&rcontext, pid, attr);
+	ret = getprocattrcon_raw(&rcontext, pid, attr, prev_context);
 
 	if (!ret) {
 		ret = selinux_raw_to_trans_context(rcontext, context);
@@ -210,45 +188,24 @@  static int getprocattrcon(char ** context,
 	return ret;
 }
 
-static int setprocattrcon_raw(const char * context,
-			      pid_t pid, const char *attr)
+static int setprocattrcon_raw(const char *context, const char *attr,
+			      char **prev_context)
 {
 	int fd;
 	ssize_t ret;
 	int errno_hold;
-	char **prev_context, *context2 = NULL;
+	char *context2 = NULL;
 
 	__selinux_once(once, init_procattr);
 	init_thread_destructor();
 
-	switch (attr[0]) {
-		case 'c':
-			prev_context = &prev_current;
-			break;
-		case 'e':
-			prev_context = &prev_exec;
-			break;
-		case 'f':
-			prev_context = &prev_fscreate;
-			break;
-		case 'k':
-			prev_context = &prev_keycreate;
-			break;
-		case 's':
-			prev_context = &prev_sockcreate;
-			break;
-		default:
-			errno = ENOENT;
-			return -1;
-	}
-
 	if (!context && !*prev_context)
 		return 0;
 	if (context && *prev_context && *prev_context != UNSET
 	    && !strcmp(context, *prev_context))
 		return 0;
 
-	fd = openattr(pid, attr, O_RDWR | O_CLOEXEC);
+	fd = openattr(0, attr, O_RDWR | O_CLOEXEC);
 	if (fd < 0)
 		return -1;
 	if (context) {
@@ -279,8 +236,8 @@  out:
 	}
 }
 
-static int setprocattrcon(const char * context,
-			  pid_t pid, const char *attr)
+static int setprocattrcon(const char *context, const char *attr,
+			  char **prev_context)
 {
 	int ret;
 	char * rcontext;
@@ -288,62 +245,58 @@  static int setprocattrcon(const char * context,
 	if (selinux_trans_to_raw_context(context, &rcontext))
 		return -1;
 
-	ret = setprocattrcon_raw(rcontext, pid, attr);
+	ret = setprocattrcon_raw(rcontext, attr, prev_context);
 
 	freecon(rcontext);
 
 	return ret;
 }
 
-#define getselfattr_def(fn, attr) \
+#define getselfattr_def(fn, attr, prev_context) \
 	int get##fn##_raw(char **c) \
 	{ \
-		return getprocattrcon_raw(c, 0, #attr); \
+		return getprocattrcon_raw(c, 0, attr, prev_context); \
 	} \
 	int get##fn(char **c) \
 	{ \
-		return getprocattrcon(c, 0, #attr); \
+		return getprocattrcon(c, 0, attr, prev_context); \
 	}
 
-#define setselfattr_def(fn, attr) \
+#define setselfattr_def(fn, attr, prev_context) \
 	int set##fn##_raw(const char * c) \
 	{ \
-		return setprocattrcon_raw(c, 0, #attr); \
+		return setprocattrcon_raw(c, attr, &prev_context); \
 	} \
 	int set##fn(const char * c) \
 	{ \
-		return setprocattrcon(c, 0, #attr); \
+		return setprocattrcon(c, attr, &prev_context); \
 	}
 
-#define all_selfattr_def(fn, attr) \
-	getselfattr_def(fn, attr)	 \
-	setselfattr_def(fn, attr)
+#define all_selfattr_def(fn, attr, prev_context) \
+	getselfattr_def(fn, attr, prev_context)	 \
+	setselfattr_def(fn, attr, prev_context)
 
-#define getpidattr_def(fn, attr) \
-	int get##fn##_raw(pid_t pid, char **c)	\
-	{ \
-		if (pid <= 0) { \
-			errno = EINVAL; \
-			return -1; \
-		} else { \
-			return getprocattrcon_raw(c, pid, #attr); \
-		} \
-	} \
-	int get##fn(pid_t pid, char **c)	\
-	{ \
-		if (pid <= 0) { \
-			errno = EINVAL; \
-			return -1; \
-		} else { \
-			return getprocattrcon(c, pid, #attr); \
-		} \
-	}
+all_selfattr_def(con, "current", prev_current)
+    getselfattr_def(prevcon, "prev", NULL)
+    all_selfattr_def(execcon, "exec", prev_exec)
+    all_selfattr_def(fscreatecon, "fscreate", prev_fscreate)
+    all_selfattr_def(sockcreatecon, "sockcreate", prev_sockcreate)
+    all_selfattr_def(keycreatecon, "keycreate", prev_keycreate)
 
-all_selfattr_def(con, current)
-    getpidattr_def(pidcon, current)
-    getselfattr_def(prevcon, prev)
-    all_selfattr_def(execcon, exec)
-    all_selfattr_def(fscreatecon, fscreate)
-    all_selfattr_def(sockcreatecon, sockcreate)
-    all_selfattr_def(keycreatecon, keycreate)
+int getpidcon_raw(pid_t pid, char **c)
+{
+	if (pid <= 0) {
+		errno = EINVAL;
+		return -1;
+	}
+	return getprocattrcon_raw(c, pid, "current", NULL);
+}
 
+int getpidcon(pid_t pid, char **c)
+{
+	if (pid <= 0) {
+		errno = EINVAL;
+		return -1;
+	}
+	return getprocattrcon(c, pid, "current", NULL);
+}