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: <CAKD1Yr3itVp4UgWF1uFSVRKzQm_vaLEQT=FnfBNjUJJVTj2Z+Q@mail.gmail.com>
Date:	Tue, 17 May 2016 11:04:53 +0900
From:	Lorenzo Colitti <lorenzo@...gle.com>
To:	Stephen Hemminger <stephen@...workplumber.org>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	David Ahern <dsa@...ulusnetworks.com>
Subject: Re: [PATCH iproute2] ss: Tell user about -EOPNOTSUPP for SOCK_DESTROY

On Tue, May 17, 2016 at 10:52 AM, David Ahern <dsa@...ulusnetworks.com> wrote:
> code is not setup to handle that. Only option seems to be at least dump an
> error message, but the message can not relate any of the specifics about the
> filter. So something like this though it dumps the message per socket
> matched by the filter. Could throttle it to once.
> [...]
>         if (diag_arg->f->kill && kill_inet_sock(h, arg) != 0) {
> -               if (errno == EOPNOTSUPP || errno == ENOENT) {
> -                       /* Socket can't be closed, or is already closed. */
> +               if (errno == ENOENT) {
> +                       /* socket is already closed. */
> +                       return 0;
> +               /* Socket can't be closed OR config is not enabled */
> +               } else if (errno == EOPNOTSUPP) {
> +                       perror("SOCK_DESTROY answers");

The reason the code was written like that is that I didn't want to
print one error message for every socket that can't be closed - such
as TIME_WAIT sockets or UDP sockets.

Given that the filter can specify a number of sockets, some of which
can and some of which can't be closed, and that whether a given socket
can be closed is only known at the time we attempt to close it, there
is a choice between two bad outcomes:

1. Users try to use "ss -K" with a kernel that doesn't support it, and
get confused about why it does nothing and doesn't print an error
message.
2. Users use "ss -K" with a kernel that does support it, and get
irritated by seeing one error message per TCP_TIME_WAIT socket, UDP
socket, etc.

Personally I think it's more important to avoid #2 than #1, because #1
is one time (only if you're compiling your own kernel), but #2 is
forever. Also, I think it's consistent with other behaviours in ss -
for example, if the kernel doesn't support SOCK_DIAG for UDP, you just
get nothing back if you run "ss -u".

That said, I'm not the maintainer of this code. Stephen, any thoughts?