Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160407094806.GE9862@port70.net>
Date: Thu, 7 Apr 2016 11:48:06 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: recvmsg/sendmsg broken on mips64

* Sebastian Gottschall <s.gottschall@...wrt.com> [2016-04-02 11:52:32 +0200]:
> i understand the reason why the datatypes are defined as is, but on the
> other hand the argument is irrelevant
> if it doesnt work portable or not. (for some reason which might be mips
> specific)
> but anyway. i dont want to discuss this to the deep and. i prefer to find
> and fix the bug by keeping the original structures.

"original structures" does not make sense.

the next glibc will fix their bug and use exactly the same struct as musl:
http://sourceware.org/ml/libc-alpha/2016-03/msg00661.html

eventually uclibc will fix this too (assuming it's still maintained)

(and hopefully at some point the kernel will introduce new syscalls
that use the correct structs.)

> i will do some debugging on this today
> 
> >
> >your fix does not explain that unless there is
> >a >4G message somewhere which i think is not
> >supported on the kernel side either.
> >
> >please send a proper bug report about what
> >breaks, it sounds like the padding is at
> >the wrong place. changing int,int to size_t
> >should make no difference for iproute2.
> it not just padding if you look at confusing codelines like this in sendmsg
> for me it look like someone creates a copy of the buffer to work with it.
> but i dont see a reason for it and is does also limit the maximum size of a
> message
> and code like this h = *msg; should be replaced by memcpy, since the
> compiler may optimize that in a bad way . i have seen compiler introduced
> bugs
> in the past on lines like that. for me that code should be removed. clearing
> padding is one thing, but why doing a copy?
> 

ok so the failure is in sendmsg and in the msg_control copy.

does the call fail with ENOMEM (because >1024 bytes of ancillary data)?
that would be easy to fix..

(libc has to make a copy, the struct is const and might be in
readonly memory. a detailed bug report of the failure would
be more useful than speculations about broken compilers..
e.g. strace log with and without the msg_control copying.)

> #if LONG_MAX > INT_MAX
>         struct msghdr h;
>         struct cmsghdr chbuf[1024/sizeof(struct cmsghdr)+1], *c;
>         if (msg) {
>                 h = *msg;
>                 h.__pad1 = h.__pad2 = 0;
>                 msg = &h;
>                 if (h.msg_controllen) {
>                         if (h.msg_controllen > 1024) {
>                                 errno = ENOMEM;
>                                 return -1;
>                         }
>                         memcpy(chbuf, h.msg_control, h.msg_controllen);
>                         h.msg_control = chbuf;
>                         for (c=CMSG_FIRSTHDR(&h); c; c=CMSG_NXTHDR(&h,c))
>                                 c->__pad1 = 0;
>                 }
>         }
> #endif

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.