[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/5] util: simplify virSetUIDGIDWithCaps



On 03/25/2013 08:25 AM, Paolo Bonzini wrote:
> The need_prctl variable is not really needed.  If it is false,
> capng_apply will be called twice with the same set, causing
> a little extra work but no problem.  This keeps the code a bit
> simpler.
> 
> It is also clearer to invoke capng_apply(CAPNG_SELECT_BOUNDS)
> separately, to make sure it is done while we have CAP_SETPCAP.
> 
> Signed-off-by: Paolo Bonzini <pbonzini redhat com>
> ---
>  src/util/virutil.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)

>      /* Change to the temp capabilities */
> -    if ((capng_ret = capng_apply(CAPNG_SELECT_BOTH)) < 0) {
> +    if ((capng_ret = capng_apply(CAPNG_SELECT_CAPS)) < 0) {

Beforehand, we limited both caps and bounding set, with an overlarge
set, now you are limiting just caps...

>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("cannot apply process capabilities %d"), capng_ret);
>          goto cleanup;
> @@ -3063,12 +3061,18 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, unsigned long long capBits,
>          goto cleanup;
>  
>      /* Tell it we are done keeping capabilities */
> -    if (need_prctl && prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0)) {
> +    if (prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0)) {
>          virReportSystemError(errno, "%s",
>                               _("prctl failed to reset KEEPCAPS"));
>          goto cleanup;
>      }
>  
> +    /* Set bounding set while we have CAP_SETPCAP.  Unfortunately we cannot
> +     * do this if we failed to get the capability above, so ignore the
> +     * return value.
> +     */
> +    capng_apply(CAPNG_SELECT_BOUNDS);

...and then separately limiting bounds, but still while having an
overlarge set.

> +
>      /* Drop the caps that allow setuid/gid (unless they were requested) */
>      if (need_setgid)
>          capng_update(CAPNG_DROP, CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETGID);
> @@ -3078,7 +3082,7 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, unsigned long long capBits,
>      if (need_setpcap)
>          capng_update(CAPNG_DROP, CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETPCAP);

Here, the set is now pruned to size...

>  
> -    if (need_prctl && ((capng_ret = capng_apply(CAPNG_SELECT_BOTH)) < 0)) {
> +    if (((capng_ret = capng_apply(CAPNG_SELECT_CAPS)) < 0)) {

...but you are now only limiting caps, not the bounding set.  Is that
correct?

Does this need to be considered for 1.0.4, or can we delay it to
post-release?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]