Date: Wed, 4 Jan 2017 22:29:52 -0800 From: Ngie Cooper <yaneurabeya@gmail.com> To: Juli Mallett <juli@clockworksquid.com> Cc: Jilles Tjoelker <jilles@stack.nl>, Ngie Cooper <ngie@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r311233 - head/contrib/netbsd-tests/fs/tmpfs Message-ID: <BC5AB049-8DE3-4B60-88AA-95F8377513E9@gmail.com> In-Reply-To: <CACVs6=9Ku1X8PG1d65XqAQ_ivpsLxF9VJy_06S%2BT-Ve%2BQN6YTw@mail.gmail.com> References: <201701040246.v042kaEh039041@repo.freebsd.org> <20170104233650.GB17765@stack.nl> <CACVs6=9Ku1X8PG1d65XqAQ_ivpsLxF9VJy_06S%2BT-Ve%2BQN6YTw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Jan 4, 2017, at 15:45, Juli Mallett <juli@clockworksquid.com> wrote: >=20 >> On Wed, Jan 4, 2017 at 3:36 PM, Jilles Tjoelker <jilles@stack.nl> wrote: >>> On Wed, Jan 04, 2017 at 02:46:36AM +0000, Ngie Cooper wrote: >>> - Initialize .sun_len before passing it to strlcpy and bind. >> It would be better to avoid naming the non-portable sun_len field if it >> is just to make Coverity happy. I suggest initializing the structure >> with designated initializers or memset(). >>=20 >> Apart from that, the value for sun_len is wrong; it should be the length >> of the whole structure and not just the sun_path part. Fortunately, the >> field is ignored by bind(), which uses the addrlen parameter instead. The problem was the strcpy and the fact that the code didn't check the input= buffer to make sure it didn't overrun the destination buffer. > This is incorrect, too. It's the length of the sockaddr_un header > plus the actual length of the pathname, not the available size of the > path field. It's kind of awful that it's inconsistent with the other > sockaddr types, but that's the fun of sockaddr_un, to accommodate the > fact that the path name is naturally a variable-length field. In > fact, the calculation here seems to be wrong, also; we have the > SUN_LEN macro in <sys/un.h> for a reason, and it's what the unix(4) > manpage suggests. Of course, sun_len is sort of needlessly obscure > and in general it's best for us to fix anything which requires the > _len fields to be accurate, and to just ignore them :( Ack.. thanks for the reminder :/.. I'll fix this soon :(. -Ngie=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BC5AB049-8DE3-4B60-88AA-95F8377513E9>