On Sun, Apr 10, 2016 at 03:09:44PM +0200, Sebastien Marie wrote:
> Hi,
>
> The following diff simplifies the check for allowing only promises
> reductions.
ping ?
> Please review it carefully: it implies several bitwise operations.
>
> I will try also to explain the diff step-by-step because even if it is
> small in size, it manipulates lot of things.
>
>
> 1. removing: flags &= p->p_p->ps_pledge;
>
> `flags' is the result of parsing user promises string. It is builded
> using pledgereq_flags() function, and will contains only the requested
> flags (no _USERSET flags).
>
> Just before, we have checked that `flags' is a subset of `ps_pledge'. So
> we could use it directly, without using ps_pledge as mask. The
> expression would set `flags' to intersection between flags list of
> promises and ps_pledge list of promises, but as `flags' is already a subset
> of `ps_pledge' it is unneeded.
>
>
> 2. removing: flags &= PLEDGE_USERSET;
>
> this one ensure that `flags' contains only bitflags in _USERSET part.
> Due to pledgereq_flags(), it is necessary the case, so it is unneeded
> and could be removed too.
>
>
> 3. remove mask to PLEDGE_USERSET in conditionnal
>
> `ps_pledge' could containing promises bitflags from user promises and
> bitflags from kernel (like PLEDGE_YPACTIVE). `flags' could only have
> user promises.
>
> comparaison of (flags|ps_pledge) & USERSET != ps_pledge & USERSET
> and (flags|ps_pledge) != ps_pledge
>
> should have some result. We still ensure that `flags' doesn't contains
> any extra bits that `ps_pledge' doesn't have.
>
> (The diff was generated in a way it doesn't depend on previous one)
>
> Comments ?
> --
> Sebastien Marie
>
>
> Index: kern/kern_pledge.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.162
> diff -u -p -r1.162 kern_pledge.c
> --- kern/kern_pledge.c 30 Mar 2016 07:49:11 -0000 1.162
> +++ kern/kern_pledge.c 10 Apr 2016 12:12:04 -0000
> @@ -437,16 +437,14 @@ sys_pledge(struct proc *p, void *v, regi
> if (flags & ~PLEDGE_USERSET)
> return (EINVAL);
>
> - if ((p->p_p->ps_flags & PS_PLEDGE)) {
> - /* Already pledged, only allow reductions */
> - if (((flags | p->p_p->ps_pledge) & PLEDGE_USERSET) !=
> - (p->p_p->ps_pledge & PLEDGE_USERSET)) {
> - return (EPERM);
> - }
> -
> - flags &= p->p_p->ps_pledge;
> - flags &= PLEDGE_USERSET; /* Relearn _ACTIVE */
> - }
> + /*
> + * if we are already pledged, allow only promises reductions.
> + * flags doesn't contain flags outside _USERSET: they will be
> + * relearned.
> + */
> + if (ISSET(p->p_p->ps_flags, PS_PLEDGE) &&
> + (((flags | p->p_p->ps_pledge) != p->p_p->ps_pledge)))
> + return (EPERM);
> }
>
> if (SCARG(uap, paths)) {
--
Sebastien Marie
