Re: iproute: ss truncates abstract unix domain socket embedding null

[Isaac Boukris]

On Thu, Oct 27, 2016 at 1:22 AM, Stephen Hemminger
 wrote:
> Just translating all null characters to @ seems the most consistent and
> logical. Also translating other all non-printing characters to something 
> (maybe '?')
> might be wise. It would be nice if all utilities output the same thing.

I've sent two patches to netdev with which all of ss, netstat and lsof
would translate all null bytes the same way.
I left out the other non-printable characters as I wasn't as confident about it.

Thanks.

Re: iproute: ss truncates abstract unix domain socket embedding null

[Stephen Hemminger]

On Wed, 26 Oct 2016 21:49:35 +0300
Isaac Boukris  wrote:

> Hi Stephen, thanks for looking into this.
> 
> On Wed, Oct 26, 2016 at 8:15 PM, Stephen Hemminger
>  wrote:
> > On Tue, 18 Oct 2016 21:46:48 +0300
> > Isaac Boukris  wrote:
> >  
> >> Hi again,
> >>
> >> On Sun, Oct 16, 2016 at 11:43 PM, Isaac Boukris  
> >> wrote:  
> >> > Hello,
> >> >
> >> > The unix(7) man page says that null have no special meaning in
> >> > abstract unix domain socket address (the length is specified
> >> > therefore).
> >> >
> >> > However, when such name (embedding null) is used, ss (and netstat)
> >> > will only show up to the first null occurrence (second technically, if
> >> > we count the null prefix).
> >> > e.g. the name "\0/tmp/fo\0.sock" is displayed as: "@/tmp/fo" (whilst
> >> > strace tool shows it as: sun_path=@"/tmp/fo\0.sock").
> >> >
> >> > Would it be more useful if it printed the whole name and escaped the 
> >> > null?
> >> > If so, would '\0' be ok for escaping the null?  
> >>
> >>
> >> Meanwhile, I've got it to escape the null character with with '\0' as 
> >> suggested.
> >> Can anyone take a look and advise if I'm on the right track? Thanks!  
> >
> > I did a little investigation and current ss behavior matches the output
> > of other commands (netstat and lsof).  Therefore I really can't see the 
> > motivation
> > to fix this.  
> 
> The motivation behind the fix is because the usage of abstract unix
> domain socket is somewhat tricky.
> I've seen it being used incorrectly where for example the addrlen was
> specified as 'sizeof(struct sockaddr_un)' which is ok for regular unix
> sockets because their names are null-terminated, but with abstract
> sockets it causes extra null padding which leads to interoperability
> problems.
> On another occasion, addrlen was incremented to account for an
> additional null-termination byte.
> 
> I was thinking therefore, it could help if the diagnostic tools would
> show all the significant bytes of the name in order to make it clear
> and easy to distinguish.
> 
> On the other hand, I think I've complicated it a little bit with the
> '\0' escaping.
> Perhaps it would suffice to substitute each null character with an '@'
> sign, the same way we do for the null prefix.
> 
> As regarding netstat, I have in fact made a patch for it, but then I
> realized it perhaps isn't its fault as it prints what it reads from
> '/proc/net/unix' which prints the null prefix as '@' but leaves
> subsequent nulls as is (literally, can be seen with 'cat -A').
> So I'm trying to see if '/proc/net/unix' can be fixed to translate all
> null occurrences to '@' sign (not only the prefix).
> This should fix netstat and also (I think) the alternative 'proc' base
> implementation in ss (unix_use_proc).
> 
> What do you think?

Just translating all null characters to @ seems the most consistent and
logical. Also translating other all non-printing characters to something (maybe 
'?')
might be wise. It would be nice if all utilities output the same thing.

Re: iproute: ss truncates abstract unix domain socket embedding null

[Isaac Boukris]

Hi Stephen, thanks for looking into this.

On Wed, Oct 26, 2016 at 8:15 PM, Stephen Hemminger
 wrote:
> On Tue, 18 Oct 2016 21:46:48 +0300
> Isaac Boukris  wrote:
>
>> Hi again,
>>
>> On Sun, Oct 16, 2016 at 11:43 PM, Isaac Boukris  wrote:
>> > Hello,
>> >
>> > The unix(7) man page says that null have no special meaning in
>> > abstract unix domain socket address (the length is specified
>> > therefore).
>> >
>> > However, when such name (embedding null) is used, ss (and netstat)
>> > will only show up to the first null occurrence (second technically, if
>> > we count the null prefix).
>> > e.g. the name "\0/tmp/fo\0.sock" is displayed as: "@/tmp/fo" (whilst
>> > strace tool shows it as: sun_path=@"/tmp/fo\0.sock").
>> >
>> > Would it be more useful if it printed the whole name and escaped the null?
>> > If so, would '\0' be ok for escaping the null?
>>
>>
>> Meanwhile, I've got it to escape the null character with with '\0' as 
>> suggested.
>> Can anyone take a look and advise if I'm on the right track? Thanks!
>
> I did a little investigation and current ss behavior matches the output
> of other commands (netstat and lsof).  Therefore I really can't see the 
> motivation
> to fix this.

The motivation behind the fix is because the usage of abstract unix
domain socket is somewhat tricky.
I've seen it being used incorrectly where for example the addrlen was
specified as 'sizeof(struct sockaddr_un)' which is ok for regular unix
sockets because their names are null-terminated, but with abstract
sockets it causes extra null padding which leads to interoperability
problems.
On another occasion, addrlen was incremented to account for an
additional null-termination byte.

I was thinking therefore, it could help if the diagnostic tools would
show all the significant bytes of the name in order to make it clear
and easy to distinguish.

On the other hand, I think I've complicated it a little bit with the
'\0' escaping.
Perhaps it would suffice to substitute each null character with an '@'
sign, the same way we do for the null prefix.

As regarding netstat, I have in fact made a patch for it, but then I
realized it perhaps isn't its fault as it prints what it reads from
'/proc/net/unix' which prints the null prefix as '@' but leaves
subsequent nulls as is (literally, can be seen with 'cat -A').
So I'm trying to see if '/proc/net/unix' can be fixed to translate all
null occurrences to '@' sign (not only the prefix).
This should fix netstat and also (I think) the alternative 'proc' base
implementation in ss (unix_use_proc).

What do you think?

Re: iproute: ss truncates abstract unix domain socket embedding null

[Stephen Hemminger]

On Tue, 18 Oct 2016 21:46:48 +0300
Isaac Boukris  wrote:

> Hi again,
> 
> On Sun, Oct 16, 2016 at 11:43 PM, Isaac Boukris  wrote:
> > Hello,
> >
> > The unix(7) man page says that null have no special meaning in
> > abstract unix domain socket address (the length is specified
> > therefore).
> >
> > However, when such name (embedding null) is used, ss (and netstat)
> > will only show up to the first null occurrence (second technically, if
> > we count the null prefix).
> > e.g. the name "\0/tmp/fo\0.sock" is displayed as: "@/tmp/fo" (whilst
> > strace tool shows it as: sun_path=@"/tmp/fo\0.sock").
> >
> > Would it be more useful if it printed the whole name and escaped the null?
> > If so, would '\0' be ok for escaping the null?  
> 
> 
> Meanwhile, I've got it to escape the null character with with '\0' as 
> suggested.
> Can anyone take a look and advise if I'm on the right track? Thanks!

I did a little investigation and current ss behavior matches the output
of other commands (netstat and lsof).  Therefore I really can't see the 
motivation
to fix this. 

Re: iproute: ss truncates abstract unix domain socket embedding null

[Isaac Boukris]

Hi again,

On Sun, Oct 16, 2016 at 11:43 PM, Isaac Boukris  wrote:
> Hello,
>
> The unix(7) man page says that null have no special meaning in
> abstract unix domain socket address (the length is specified
> therefore).
>
> However, when such name (embedding null) is used, ss (and netstat)
> will only show up to the first null occurrence (second technically, if
> we count the null prefix).
> e.g. the name "\0/tmp/fo\0.sock" is displayed as: "@/tmp/fo" (whilst
> strace tool shows it as: sun_path=@"/tmp/fo\0.sock").
>
> Would it be more useful if it printed the whole name and escaped the null?
> If so, would '\0' be ok for escaping the null?


Meanwhile, I've got it to escape the null character with with '\0' as suggested.
Can anyone take a look and advise if I'm on the right track? Thanks!


diff --git a/misc/ss.c b/misc/ss.c
index dd77b81..3e41f44 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2869,7 +2869,7 @@ static int unix_show_sock(const struct
sockaddr_nl *addr, struct nlmsghdr *nlh,
struct filter *f = (struct filter *)arg;
struct unix_diag_msg *r = NLMSG_DATA(nlh);
struct rtattr *tb[UNIX_DIAG_MAX+1];
-   char name[128];
+   char name[128*2];
struct sockstat stat = { .name = "*", .peer_name = "*" };

parse_rtattr(tb, UNIX_DIAG_MAX, (struct rtattr *)(r+1),
@@ -2891,11 +2891,25 @@ static int unix_show_sock(const struct
sockaddr_nl *addr, struct nlmsghdr *nlh,
}
if (tb[UNIX_DIAG_NAME]) {
int len = RTA_PAYLOAD(tb[UNIX_DIAG_NAME]);
+   char *real_name = RTA_DATA(tb[UNIX_DIAG_NAME]);

-   memcpy(name, RTA_DATA(tb[UNIX_DIAG_NAME]), len);
-   name[len] = '\0';
-   if (name[0] == '\0')
+   if (real_name[0] == '\0') {
+   int i, j;
name[0] = '@';
+   for (i = j = 1; i < len; ++i) {
+   if (real_name[i] == '\0') {
+   name[j++] = '\\';
+   name[j++] = '0';
+   }
+   else
+   name[j++] = real_name[i];
+   }
+   name[j] = '\0';
+   } else {
+   memcpy(name, real_name, len);
+   name[len] = '\0';
+   }
+
stat.name = [0];
memcpy(stat.local.data, , sizeof(stat.name));
}