On Fri, Sep 09, 2016 at 02:23:17PM -0700, Joe Stringer wrote:
> On 9 September 2016 at 14:05, Ben Pfaff <b...@ovn.org> wrote:
> > On Fri, Sep 09, 2016 at 01:47:01PM -0700, Joe Stringer wrote:
> >> On 19 July 2016 at 20:08, Ben Pfaff <b...@ovn.org> wrote:
> >> > On Tue, Jul 19, 2016 at 05:05:51PM -0300, Thadeu Lima de Souza Cascardo
> >> > wrote:
> >> >> FreeBSD returns a socklen of sockaddr_storage when doing an accept on
> >> >> an unix
> >> >> STREAM socket. The current code will assume it means a sun_path larger
> >> >> than 0.
> >> >>
> >> >> That breaks some tests like the one below which don't expect to find
> >> >> "unix::" on
> >> >> the logs.
> >> >>
> >> >> As a Linux abstract address would not have a more useful name either,
> >> >> it's
> >> >> better to check that sun_path starts with a non-zero byte and return 0
> >> >> length in
> >> >> case it doesn't.
> >> >>
> >> >> 402: ovs-ofctl replace-flows with --bundle FAILED
> >> >> (ovs-ofctl.at:2928)
> >> >> 2016-07-08T12:44:30.068Z|00020|vconn|DBG|unix:: sent (Success):
> >> >> OFPT_HELLO (OF1.6) (xid=0x1):
> >> >>
> >> >> Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@redhat.com>
> >> >
> >> > Applied, thanks!
> >>
> >> For what it's worth, this introduces a warning on my Linux x86_64
> >> machine with valgrind:
> >>
> >> ==18725== Conditional jump or move depends on uninitialised value(s)
> >> ==18725== at 0x45BF65: get_unix_name_len (socket-util-unix.c:392)
> >> ==18725== by 0x45C842: punix_accept (stream-unix.c:113)
> >> ==18725== by 0x46897B: pfd_accept (stream-fd.c:263)
> >> ==18725== by 0x44CF6F: pstream_accept (stream.c:557)
> >> ==18725== by 0x44FE6C: unixctl_server_run (unixctl.c:385)
> >> ==18725== by 0x4081AC: main_loop (ovsdb-server.c:182)
> >> ==18725== by 0x406432: main (ovsdb-server.c:429)
> >
> > Does this help?
> >
> > diff --git a/lib/socket-util-unix.c b/lib/socket-util-unix.c
> > index 5d4b88c..a7a7e03 100644
> > --- a/lib/socket-util-unix.c
> > +++ b/lib/socket-util-unix.c
> > @@ -389,7 +389,7 @@ error:
> > int
> > get_unix_name_len(const struct sockaddr_un *sun, socklen_t sun_len)
> > {
> > - return (sun_len >= offsetof(struct sockaddr_un, sun_path) &&
> > + return (sun_len > offsetof(struct sockaddr_un, sun_path) &&
> > sun->sun_path[0] != 0
> > ? sun_len - offsetof(struct sockaddr_un, sun_path)
> > : 0);
>
> Yup, I don't see the valgrind warning with this change applied.
Thanks for testing.
I sent this formally for review:
https://patchwork.ozlabs.org/patch/670199/
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
