netdev - Re: [PATCH iproute2] ss: Tell user about -EOPNOTSUPP for SOCK

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: <CAKD1Yr08Zm55=oxMWUxL5-=D-gt4Hg5nfKSH7KRi38Ki=VyqcQ@mail.gmail.com>
Date:	Tue, 17 May 2016 10:01:48 +0900
From:	Lorenzo Colitti <lorenzo@...gle.com>
To:	David Ahern <dsa@...ulusnetworks.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2] ss: Tell user about -EOPNOTSUPP for SOCK_DESTROY

On Tue, May 17, 2016 at 8:53 AM, David Ahern <dsa@...ulusnetworks.com> wrote:
> @@ -2264,7 +2264,7 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
>         if (!(diag_arg->f->families & (1 << r->idiag_family)))
>                 return 0;
>         if (diag_arg->f->kill && kill_inet_sock(h, arg) != 0) {
> -               if (errno == EOPNOTSUPP || errno == ENOENT) {
> +               if (errno == ENOENT) {
>                         /* Socket can't be closed, or is already closed. */
>                         return 0;
>                 } else {

I don't think you can do this without breaking the functionality of -K.

The else branch will cause show_one_inet_sock to return -1, which will
cause rtnl_dump_filter to abort and not close any other sockets that
the user requested killing. That's incorrect, because getting
EOPNOTSUPP on one socket doesn't necessarily mean we'll get EOPNOTSUPP
on any future sockets in the same dump.

For example, EOPNOTSUPP can just mean "this socket can't be closed
because it's a timewait or NEW_SYN_RECV socket". In hindsight it might
have been better to return EBADFD in those cases, but that still
doesn't solve the UI problem. If the user does something like "ss -K
dport = :443", the user would expect the command to kill all TCP
sockets and not just abort if there happens to be a UDP socket to port
443 (which can't be closed because UDP doesn't currently implement
SOCK_DESTROY).