linux-kernel - Re: [PATCH] unix: escape all null bytes in abstract unix domain socket

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAC-fF8QA6KwXGsL2dx6PpQLqc0rDH2y57rVcRL7N-xAnVa3M6A@mail.gmail.com>
Date:   Tue, 1 Nov 2016 02:56:59 +0200
From:   Isaac Boukris <iboukris@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] unix: escape all null bytes in abstract unix domain socket

Hi David, thanks for looking at it.

On Mon, Oct 31, 2016 at 9:31 PM, David Miller <davem@...emloft.net> wrote:
> From: Isaac Boukris <iboukris@...il.com>
> Date: Sat, 29 Oct 2016 22:20:20 +0300
>
>> Abstract unix domain socket may embed null characters,
>> these should be translated to '@' when printed out to
>> proc the same way the null prefix is currently being
>> translated.
>>
>> This helps for tools such as netstat, lsof and the proc
>> based implementation in ss to show all the significant
>> bytes of the name (instead of getting cut at the first
>> null occurrence).
>>
>> Signed-off-by: Isaac Boukris <iboukris@...il.com>
>  ...
>> @@ -2805,14 +2805,19 @@ static int unix_seq_show(struct seq_file *seq, void *v)
>>
>>                       i = 0;
>>                       len = u->addr->len - sizeof(short);
>> -                     if (!UNIX_ABSTRACT(s))
>> +                     if (!UNIX_ABSTRACT(s)) {
>>                               len--;
>> -                     else {
>> +                             for ( ; i < len; i++)
>> +                                     seq_putc(seq,
>> +                                              u->addr->name->sun_path[i]);
>> +                     } else {
>>                               seq_putc(seq, '@');
>>                               i++;
>> +                             for ( ; i < len; i++)
>> +                                     seq_putc(seq,
>> +                                              u->addr->name->sun_path[i] ?:
>> +                                              '@');
>>                       }
>> -                     for ( ; i < len; i++)
>> -                             seq_putc(seq, u->addr->name->sun_path[i]);
>
> I think this patch is simpler if you just do the "@" translation
> unconditionally, if it'll never trigger for the !UNIX_ABSTRACT case
> that is perfectly fine.

I've sent an updated patch.
Logically now, the 'else' block just above could be removed, but it
isn't obvious from the code that 'sun_path[0] == 0' so I left it as
is.