Question regarding alignment patch - openssh

This is a discussion on Question regarding alignment patch - openssh ; Contrast http://cvsweb.mindrot.org/index.cgi/...1=1.23;r2=1.24 with http://www.openbsd.org/cgi-bin/cvswe...1=1.14&r2=1.15 The original replaces cmsgbuf.tmp with cmsgbuf.buf, while the -portable version *adds* cmsgbuf.buf but retains cmsgbuf.tmp. I assume this was an oversight, and cmsgbuf.tmp should be removed? DES -- Dag-Erling Smørgrav - des@des.no _______________________________________________ openssh-unix-dev mailing list ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: Question regarding alignment patch

  1. Question regarding alignment patch

    Contrast

    http://cvsweb.mindrot.org/index.cgi/...1=1.23;r2=1.24
    with
    http://www.openbsd.org/cgi-bin/cvswe...1=1.14&r2=1.15

    The original replaces cmsgbuf.tmp with cmsgbuf.buf, while the -portable
    version *adds* cmsgbuf.buf but retains cmsgbuf.tmp.

    I assume this was an oversight, and cmsgbuf.tmp should be removed?

    DES
    --
    Dag-Erling Smørgrav - des@des.no
    _______________________________________________
    openssh-unix-dev mailing list
    openssh-unix-dev@mindrot.org
    https://lists.mindrot.org/mailman/li...enssh-unix-dev

  2. Re: Question regarding alignment patch

    On Tue, Jul 29, 2008 at 04:53:38PM +0200, Dag-Erling Smrgrav wrote:
    > Contrast
    >
    > http://cvsweb.mindrot.org/index.cgi/...1=1.23;r2=1.24
    > with
    > http://www.openbsd.org/cgi-bin/cvswe...1=1.14&r2=1.15
    >
    > The original replaces cmsgbuf.tmp with cmsgbuf.buf, while the -portable
    > version *adds* cmsgbuf.buf but retains cmsgbuf.tmp.
    >
    > I assume this was an oversight, and cmsgbuf.tmp should be removed?


    Yes it certainly looks like an oversight.

    On a somewhat related note, it seems that some platforms (NetBSD 1.6
    at least) fail the recvmsg with EAGAIN making multiplexing fail with:

    mm_receive_fd: recvmsg: Resource temporarily unavailable

    Retrying on EAGAIN makes this work (well, pass the regress tests
    anyway).

    Index: monitor_fdpass.c
    ================================================== =================
    RCS file: /usr/local/src/security/openssh/cvs/openssh/monitor_fdpass.c,v
    retrieving revision 1.26
    diff -u -p -r1.26 monitor_fdpass.c
    --- monitor_fdpass.c 27 Mar 2008 00:01:15 -0000 1.26
    +++ monitor_fdpass.c 30 Jul 2008 00:31:16 -0000
    @@ -51,7 +51,6 @@ mm_send_fd(int sock, int fd)
    #ifndef HAVE_ACCRIGHTS_IN_MSGHDR
    union {
    struct cmsghdr hdr;
    - char tmp[CMSG_SPACE(sizeof(int))];
    char buf[CMSG_SPACE(sizeof(int))];
    } cmsgbuf;
    struct cmsghdr *cmsg;
    @@ -76,7 +75,9 @@ mm_send_fd(int sock, int fd)
    msg.msg_iov = &vec;
    msg.msg_iovlen = 1;

    - if ((n = sendmsg(sock, &msg, 0)) == -1) {
    + while ((n = sendmsg(sock, &msg, 0)) == -1 && errno == EAGAIN)
    + ;
    + if (n == -1) {
    error("%s: sendmsg(%d): %s", __func__, fd,
    strerror(errno));
    return -1;
    @@ -124,7 +125,9 @@ mm_receive_fd(int sock)
    msg.msg_controllen = sizeof(cmsgbuf.buf);
    #endif

    - if ((n = recvmsg(sock, &msg, 0)) == -1) {
    + while ((n = recvmsg(sock, &msg, 0)) == -1 && errno == EAGAIN)
    + ;
    + if (n == -1) {
    error("%s: recvmsg: %s", __func__, strerror(errno));
    return -1;
    }

    --
    Darren Tucker (dtucker at zip.com.au)
    GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
    usually comes from bad judgement.
    _______________________________________________
    openssh-unix-dev mailing list
    openssh-unix-dev@mindrot.org
    https://lists.mindrot.org/mailman/li...enssh-unix-dev


  3. Re: Question regarding alignment patch

    On Wed, Jul 30, 2008 at 10:34:06AM +1000, Darren Tucker wrote:
    > On Tue, Jul 29, 2008 at 04:53:38PM +0200, Dag-Erling Smrgrav wrote:
    > > Contrast
    > >
    > > http://cvsweb.mindrot.org/index.cgi/...1=1.23;r2=1.24
    > > with
    > > http://www.openbsd.org/cgi-bin/cvswe...1=1.14&r2=1.15
    > >
    > > The original replaces cmsgbuf.tmp with cmsgbuf.buf, while the -portable
    > > version *adds* cmsgbuf.buf but retains cmsgbuf.tmp.
    > >
    > > I assume this was an oversight, and cmsgbuf.tmp should be removed?

    >
    > Yes it certainly looks like an oversight.
    >
    > On a somewhat related note, it seems that some platforms (NetBSD 1.6
    > at least) fail the recvmsg with EAGAIN making multiplexing fail with:


    On another related note, if we reshuffle some variable declarations and
    fiddle spacing we can cut the diff against OpenBSD by quite a bit (55
    lines to 38 lines) and make the remainder more readable (this conficts
    with my previous patch).

    I'm not sure if there's a reason for the existing order (can't see one
    from the CVS history). Damien?

    Index: monitor_fdpass.c
    ================================================== =================
    RCS file: /usr/local/src/security/openssh/cvs/openssh/monitor_fdpass.c,v
    retrieving revision 1.26
    diff -u -p -r1.26 monitor_fdpass.c
    --- monitor_fdpass.c 27 Mar 2008 00:01:15 -0000 1.26
    +++ monitor_fdpass.c 30 Jul 2008 00:37:37 -0000
    @@ -45,17 +45,16 @@ mm_send_fd(int sock, int fd)
    {
    #if defined(HAVE_SENDMSG) && (defined(HAVE_ACCRIGHTS_IN_MSGHDR) || defined(HAVE_CONTROL_IN_MSGHDR))
    struct msghdr msg;
    - struct iovec vec;
    - char ch = '\0';
    - ssize_t n;
    #ifndef HAVE_ACCRIGHTS_IN_MSGHDR
    union {
    struct cmsghdr hdr;
    - char tmp[CMSG_SPACE(sizeof(int))];
    char buf[CMSG_SPACE(sizeof(int))];
    } cmsgbuf;
    struct cmsghdr *cmsg;
    #endif
    + struct iovec vec;
    + char ch = '\0';
    + ssize_t n;

    memset(&msg, 0, sizeof(msg));
    #ifdef HAVE_ACCRIGHTS_IN_MSGHDR
    @@ -99,10 +98,6 @@ mm_receive_fd(int sock)
    {
    #if defined(HAVE_RECVMSG) && (defined(HAVE_ACCRIGHTS_IN_MSGHDR) || defined(HAVE_CONTROL_IN_MSGHDR))
    struct msghdr msg;
    - struct iovec vec;
    - ssize_t n;
    - char ch;
    - int fd;
    #ifndef HAVE_ACCRIGHTS_IN_MSGHDR
    union {
    struct cmsghdr hdr;
    @@ -110,6 +105,10 @@ mm_receive_fd(int sock)
    } cmsgbuf;
    struct cmsghdr *cmsg;
    #endif
    + struct iovec vec;
    + ssize_t n;
    + char ch;
    + int fd;

    memset(&msg, 0, sizeof(msg));
    vec.iov_base = &ch;
    @@ -128,6 +127,7 @@ mm_receive_fd(int sock)
    error("%s: recvmsg: %s", __func__, strerror(errno));
    return -1;
    }
    +
    if (n != 1) {
    error("%s: recvmsg: expected received 1 got %ld",
    __func__, (long)n);
    @@ -145,6 +145,7 @@ mm_receive_fd(int sock)
    error("%s: no message header", __func__);
    return -1;
    }
    +
    #ifndef BROKEN_CMSG_TYPE
    if (cmsg->cmsg_type != SCM_RIGHTS) {
    error("%s: expected type %d got %d", __func__,

    --
    Darren Tucker (dtucker at zip.com.au)
    GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
    usually comes from bad judgement.
    _______________________________________________
    openssh-unix-dev mailing list
    openssh-unix-dev@mindrot.org
    https://lists.mindrot.org/mailman/li...enssh-unix-dev


  4. Re: Question regarding alignment patch

    On Wed, 30 Jul 2008, Darren Tucker wrote:

    > On another related note, if we reshuffle some variable declarations and
    > fiddle spacing we can cut the diff against OpenBSD by quite a bit (55
    > lines to 38 lines) and make the remainder more readable (this conficts
    > with my previous patch).
    >
    > I'm not sure if there's a reason for the existing order (can't see one
    > from the CVS history). Damien?


    No reason, feel free to commit your fixes.

    -d
    _______________________________________________
    openssh-unix-dev mailing list
    openssh-unix-dev@mindrot.org
    https://lists.mindrot.org/mailman/li...enssh-unix-dev


+ Reply to Thread