Re: [chrony-dev] [PATCH] Privilege Separation - Version 3 - Add helper process

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Fri, Nov 20, 2015 at 10:13:52PM +1300, Bryan Christianson wrote:
> 
> > On 20/11/2015, at 8:49 PM, Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:
> > 
> > On Fri, Nov 20, 2015 at 03:35:24PM +1300, Bryan Christianson wrote:
> > 
> >> +static int
> >> +send_response(int fd, const PrvResponse *res)
> >> +{
> >> +  if (write(fd, res, sizeof(PrvResponse)) != sizeof(PrvResponse))
> >> +    return -1;
> > 
> > Can you please rework the return codes to follow the convention of 1
> > for success and 0 for fail as in the rest of the chrony code?
> 
> I think that PRV_AdjustTime, PRV_BindSocket, PRV_SetTime should return -1 on error otherwise the caller will have to handle 2 different return conventions, depending on whether its compiled to use the helper or the native call.

I meant that just for the private functions. For PRV_* functions I
agree that would make no sense.

> >> +  if (req->op == op_BINDSOCKET) {
> >> +    /* extract transferred descriptor */
> >> +    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> >> +      if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS)
> >> +        memcpy(&req->u.bind_sock.sock, CMSG_DATA(cmsg), sizeof(int));
> >> +    }
> > 
> > I think there should be a check if the socket was really set. If the
> > request didn't have a SCM_RIGHTS control message, bind() would be
> > called on a invalid descriptor.
> 
> Should this be a fatal error? I was assuming that bind() would fail and then errno on the reponse would report the error back to the daemon.

I think it doesn't have to a fatal reply to the daemon. Just returning
from the function as if the read failed so the helper exits would be
fine.

BTW, receive_from_daemon() can now check the size of the request, it
doesn't have to be in helper_main().

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.