mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 71947: Handled embedded null bytes in abstract domain socket names.
Date Fri, 10 Jan 2020 01:40:30 GMT


> On Jan. 6, 2020, 12:59 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/include/process/address.hpp
> > Lines 238 (patched)
> > <https://reviews.apache.org/r/71947/diff/1/?file=2191675#file2191675line238>
> >
> >     As discussed offline, let's add a `CHECK` here that `sun_path` is not empty.

As it turns out, I was mistaken in our offline discussion: a single null byte is a perfectly
valid name for an abstract domain socket.


> On Jan. 6, 2020, 12:59 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/include/process/address.hpp
> > Line 232 (original), 247 (patched)
> > <https://reviews.apache.org/r/71947/diff/1/?file=2191675#file2191675line247>
> >
> >     For unnamed sockets we would now return an empty string here which seems leaky.
What do you think about changing the return type of `Address::path` to e.g., `Option<string>`
instead and returning a `None` in that case?

I've thought about it and an empty string actually uniquely identifies unnamed sockets. Pathname
sockets always must have at least one character, and the difference between an empty string
and an abstract domain socket whose name is a single null byte would be that the former has
size 0, and the latter has size 1.

So I think as long as we document this correspondence, this should be fine.


- Benno


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71947/#review219127
-----------------------------------------------------------


On Jan. 10, 2020, 1:37 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71947/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2020, 1:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Address handling code for unix domain sockets assumed that
> strlen() could be used to compute the name of a unix domain
> socket, but that fails for unnamed sockets or in the case
> where an abstract domain socket contains embedded null bytes.
> 
> This patch adds a new `length` parameter to correctly handle
> these special cases.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/address.hpp 749498056b52b916dfaf6c85f83ecc05e0d5406c

>   3rdparty/libprocess/include/process/network.hpp 8f48a4a78557309a9b1b00d7defb45eed454b077

> 
> 
> Diff: https://reviews.apache.org/r/71947/diff/2/
> 
> 
> Testing
> -------
> 
> Ran existing unit tests and verified that the newly added `CHECK()` doesn't trigger.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message