Sorry to keep flogging this particular horse, but in re-reading the docs for this option as I'm trying to clean it up...
"NB: _option_value_ must be always big enough to hold sizeof(fd_t) bytes no matter how small the identity length is." There are few issues here - Prior to commit 45c681 <https://github.com/zeromq/libzmq/commit/45c68154460b5cc828cb7ac027e5407776bff2ca> there was no validation that option_len was sufficient to hold sizeof(fd_t), so in the 'no matter how small' case where the supplied identity length was less than sizeof(fd_t), which is not something that is part of the public API, so the user has to 'just know', you may get silent stack/heap corruption. With the the check, zmq_getsockopt(ZMQ_IDENTITY_FD, ...) will now return EINVAL and set option_len to the minimum required size (I think this is better than the risk of silent corruption). Now you have the following - In the case where ZeroMQ assigns a default identity, it will have a length of 5, and on a Posix system, the FD will be an int, sizeof(int) < 5, things are fine. On 64bit Windows however, sizeof(fd_t) = 8. If I set option_len to 8 on call, but only have 5 bytes, the blob will be constructed with whatever 8 bytes of data happen to be in option_value, which will then fail to match a valid identity (std::string can contain any char, is not null terminated). How would this option ever work in this case? Now, say a *nix user calls zmq_setsockopt(ZMQ_IDENTITY, ...) and uses two byte socket identities. You get the same problem trying to pass sizeof(int) as option_len in that you create a bogus identity to check. How would this option ever work in this case? The only 'safe' option seems to be blindly assume that the caller passed something at least 4 or 8 bytes long, as the code did prior to 45c681 <https://github.com/zeromq/libzmq/commit/45c68154460b5cc828cb7ac027e5407776bff2ca>, but then there is NO check against buffer overrun. Default alignment would likely save us most of the time here, but it makes my spider sense tingle and if the caller for some reason passed a pointer into a packed struct, they would not be happy with the result. On Thu, Jan 8, 2015 at 9:16 AM, Thomas Rodgers <[email protected]> wrote: > yes. > > using blob_t = std::basic_string<unsigned char>; > > char buf[8] = { 'f', 'o', 'o', 0, 0, 0, 0, 0 }; > blob_t a("foo"); > blob_t b(buf, 8); > > assert(a == b); // fails > > I think there is another issue with all of this. By default (IIRC) the > identity is 5 bytes, so on a 64 bit platform, the passed in identity would > never be sufficient to receive sizeof(fd_t) if fd_t was in fact defined as > a UINT_PTR. I don't have a Windows machine handy so I can't verify what > sizeof(SOCKET) returns, but as many of Window's 'handle' types are in fact > pointers, I suspect this might also be an issue when fd_t is typedef'd to > SOCKET. On Posix systems, it's fine. > > > > > > > On Thu, Jan 8, 2015 at 8:28 AM, Peter Kleiweg <[email protected]> wrote: > >> Thomas Rodgers schreef op de 8e dag van de louwmaand van het jaar 2015: >> >> > This is an oddball API choice (and there is a bug in the >> implementation), >> > in that option_value* is being used as both and input and an output >> > parameter. >> > >> > The size you pass in must be be *at least* sizeof(fd_t) bytes, because >> it >> > will overwrite the supplied identity string with the resulting file >> > descriptor. The bug is, it does not check to see that the size of >> supplied >> > output is sufficient to hold sizeof(fd_t), so bad things (stack/heap >> > corruption) would happen if you actually passed option_len = 2. >> >> But option_len is used for retrieving the identity string: >> >> blob_t identity= blob_t((unsigned char*)optval_,*optvallen_); >> >> Won't I get a wrong 'blob' if I use option_len = 8? >> >> >> > >> > On Thu, Jan 8, 2015 at 7:26 AM, Peter Kleiweg <[email protected]> >> wrote: >> > >> > > >> > > Suppose the identity string is only two bytes long, I pad with >> > > zeros to get a string of eight bytes. What should the value of >> > > option_len be, 2 or 8? >> >> >> >> -- >> Peter Kleiweg >> http://pkleiweg.home.xs4all.nl/ >> _______________________________________________ >> zeromq-dev mailing list >> [email protected] >> http://lists.zeromq.org/mailman/listinfo/zeromq-dev >> > >
_______________________________________________ zeromq-dev mailing list [email protected] http://lists.zeromq.org/mailman/listinfo/zeromq-dev
