Re: [PATCH] af_unix: Escape abstract unix socket address

[Yann Droneaud]

Hi,

Le 06/04/2022 à 23:59, Stephen Hemminger a écrit :
> On Wed,  6 Apr 2022 12:22:13 +0200
> Yann Droneaud <ydroneaud@xxxxxxxxxx> wrote:
>
> > Abstract unix socket address are bytes sequences up to
> > 108 bytes (UNIX_PATH_MAX == sizeof(struct sockaddr_un) -
> > offsetof(struct sockaddr_un, sun_path)).
> >
> > As with any random string of bytes, printing them in
> > /proc/net/unix should be done with caution to prevent
> > misbehavior.
> >
> > It would have been great to use seq_escape_mem() to escape
> > the control characters in a reversible way.
> >
> > Unfortunately userspace might expect that NUL bytes are
> > replaced with '@' characters as it's done currently.
> >
> > So this patch implements the following scheme: any control
> > characters, including NUL, in the abstract unix socket
> > addresses is replaced by '@' characters.
> >
> > Sadly, with such non reversible escape scheme, abstract
> > addresses such as "\0\0", "\0\a", "\0\b", "\0\t", etc.
> > will have the same representation: "@@".
> >
> > But will prevent "cat /proc/net/unix" from messing with
> > terminal, and will prevent "\n" in abstract address from
> > messing with parsing the list of Unix sockets.
> >
> > Signed-off-by: Yann Droneaud <ydroneaud@xxxxxxxxxx>
> > ---
> >   net/unix/af_unix.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > index e71a312faa1e..8021efd92301 100644
> > --- a/net/unix/af_unix.c
> > +++ b/net/unix/af_unix.c
> > @@ -3340,7 +3340,8 @@ static int unix_seq_show(struct seq_file *seq, void *v)
> >   				i++;
> >   			}
> >   			for ( ; i < len; i++)
> > -				seq_putc(seq, u->addr->name->sun_path[i] ?:
> > +				seq_putc(seq, !iscntrl(u->addr->name->sun_path[i]) ?
> > +					 u->addr->name->sun_path[i] :
> >   					 '@');
> >   		}
> >   		unix_state_unlock(s);
> Unfortunately, you will break userspace ABI with this.

It's a wanted side effect.

Consider the following program


#include <stddef.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>

#define ADDRESS "\0\n0000000000000000: 00000003 00000000 00000000 0001 03 1234567890 /bin/true"

int main(void)
{
	static const struct sockaddr_un un = {
		.sun_family = AF_UNIX,
		.sun_path = ADDRESS,
	};
	int s;

	s = socket(AF_UNIX, SOCK_STREAM, 0);
	if (s < 0) {
		perror("socket");
		return 1;
	}

	if (bind(s, (const struct sockaddr *)&un, offsetof(struct sockaddr_un,sun_path) + sizeof(ADDRESS) - 1) < 0) {
		perror("bind");
		return 1;
	}

	while (1)
		pause();

	return 0;
}


This confuses
- cat /proc/net/unix
- netstat -x

Only ss -xl doesn't take /bin/true as a Unix socket (but ss output is broken because it doesn't escape \n in unix addresses)


Regards.

--
Yann Droneaud
OPTEYA