[Rpm-maint] [PATCH 1/2] selinux: reopen label between transactions if necessary (RhBug: 746073)

[Panu Matilainen]

On 12/22/2011 04:50 PM, Ales Kozumplik wrote:
> ---
>   lib/rpmts.c          |   18 +++++++++++++++---
>   lib/rpmts_internal.h |    8 +++++---
>   lib/transaction.c    |   15 +++++++++------
>   plugins/sepolicy.c   |    2 +-
>   system.h             |    1 +
>   5 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/lib/rpmts.c b/lib/rpmts.c
> index 25ce83d..8641149 100644
> --- a/lib/rpmts.c
> +++ b/lib/rpmts.c
> @@ -738,19 +738,28 @@ struct selabel_handle * rpmtsSELabelHandle(rpmts ts)
>       return NULL;
>   }
>
> -rpmRC rpmtsSELabelInit(rpmts ts, const char *path)
> +rpmRC rpmtsSELabelInit(rpmts ts, int open_status, const char *path)
>   {
>   #if WITH_SELINUX
>       if (ts == NULL || path == NULL) {
>   	return RPMRC_FAIL;
>       }
>
> +    if (open_status) {
> +	selinux_status_close();
> +	if (selinux_status_open(0)<  0) {
> +	    return RPMRC_FAIL;
> +	}
> +    } else if (!selinux_status_updated()&&  ts->selabelHandle) {
> +	return RPMRC_OK;
> +    }
> +
>       struct selinux_opt opts[] = {
>   	{SELABEL_OPT_PATH, path}
>       };
>
>       if (ts->selabelHandle) {
> -	rpmtsSELabelFini(ts);
> +	rpmtsSELabelFini(ts, 0);
>       }
>       ts->selabelHandle = selabel_open(SELABEL_CTX_FILE, opts, 1);
>
> @@ -761,13 +770,16 @@ rpmRC rpmtsSELabelInit(rpmts ts, const char *path)
>       return RPMRC_OK;
>   }
>
> -void rpmtsSELabelFini(rpmts ts)
> +void rpmtsSELabelFini(rpmts ts, int close_status)
>   {
>   #if WITH_SELINUX
>       if (ts&&  ts->selabelHandle) {
>   	selabel_close(ts->selabelHandle);
>   	ts->selabelHandle = NULL;
>       }
> +    if (close_status) {
> +	selinux_status_close();
> +    }
>   #endif
>   }
>
> diff --git a/lib/rpmts_internal.h b/lib/rpmts_internal.h
> index 438fd46..b4213fe 100644
> --- a/lib/rpmts_internal.h
> +++ b/lib/rpmts_internal.h
> @@ -70,7 +70,7 @@ struct rpmts_s {
>   #ifdef __cplusplus
>   extern "C" {
>   #endif
> -
> +

Stray whitespace change there...

>   RPM_GNUC_INTERNAL
>   tsMembers rpmtsMembers(rpmts ts);
>
> @@ -91,16 +91,18 @@ struct selabel_handle * rpmtsSELabelHandle(rpmts ts);
>   /** \ingroup rpmts
>    * Initialize selabel
>    * @param ts		transaction set
> + * @param open_status   if the func should open selinux status or just check it
>    * @param path		path to contexts file
>    * @return		RPMRC_OK on success, RPMRC_FAIL otherwise
>    */
> -rpmRC rpmtsSELabelInit(rpmts ts, const char * path);
> +rpmRC rpmtsSELabelInit(rpmts ts, int open_status, const char * path);
>
>   /** \ingroup rpmts
>    * Clean up selabel
>    * @param ts		transaction set
> + * @param close_status  whether we should close selinux status
>    */
> -void rpmtsSELabelFini(rpmts ts);
> +void rpmtsSELabelFini(rpmts ts, int close_status);
>
>   #ifdef __cplusplus
>   }
> diff --git a/lib/transaction.c b/lib/transaction.c
> index 88219b7..b811c90 100644
> --- a/lib/transaction.c
> +++ b/lib/transaction.c
> @@ -1254,12 +1254,11 @@ static int rpmtsSetup(rpmts ts, rpmprobFilterFlags ignoreSet)
>   	(void) rpmtsSetFlags(ts, (rpmtsFlags(ts) | _noTransScripts | _noTransTriggers | RPMTRANS_FLAG_NOCOLLECTIONS));
>
>       /* if SELinux isn't enabled, init fails or test run, don't bother... */
> -    if (!is_selinux_enabled() || (rpmtsFlags(ts)&  RPMTRANS_FLAG_TEST)) {
> +    if (!is_selinux_enabled() || (rpmtsFlags(ts)&  RPMTRANS_FLAG_TEST) ||
> +	(rpmtsFlags(ts)&  RPMTRANS_FLAG_NOCONTEXTS) ||
> +	rpmtsSELabelInit(ts, 1, selinux_file_context_path())) {

This logic here is backwards: it'll now only call rpmtsSELabelInit() 
when selinux is *disabled* (by transaction flags or system-wide state).

I'd leave the current disable logic (ie "should we even try to do 
selinux stuff?") as it is, and instead check for rpmtsSELabelInit() 
return code for failure. At rpmtsSetup() we could even abort the entire 
transaction if it fails, which might actually be the right thing to do 
there anyway (user can always override with --nocontexts if they know 
better than us).

	- Panu -