Re: [PATCH iproute2 v2] ss: drop the /proc parsers for packet and netlink sockets

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed,  2 Sep 2026 03:56:33 +0000
Prabhakar Pujeri <prabhakar.pujeri@dell.com> wrote:

> packet_show() and netlink_show() fall back to parsing
> /proc/net/packet and /proc/net/netlink only when the sock_diag
> netlink dump fails.  That fallback exists for kernels without
> sock_diag support and parses each record with sscanf() without
> checking the conversion count, so truncated or malformed lines
> leave fields used by filtering and printing uninitialized.
> 
> Remove both sscanf() based parsers instead of trying to validate
> them; kernels new enough to matter answer the sock_diag request.
> packet_show_line() and the PROC_NET_PACKET/PROC_NET_NETLINK hooks
> have no other users.
> 
> Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
> ---

This makes sense and looks good to me, but AI review spotted some nits:

Subject: Re: [PATCH iproute2] ss: drop the /proc parsers for packet and netlink sockets

On Wed, Sep 2 2026, Prabhakar Pujeri wrote:
> packet_show() and netlink_show() fall back to parsing
> /proc/net/packet and /proc/net/netlink only when the sock_diag
> netlink dump fails.  That fallback exists for kernels without
> sock_diag support and parses each record with sscanf() without
> checking the conversion count, so truncated or malformed lines
> leave fields used by filtering and printing uninitialized.

The direction is right and the diff is mostly mechanical, but the patch
stops one hunk short: it removes the fallback without removing the code
that exists only to *trigger* the fallback.  That leaves a real behaviour
regression.  Details below, plus two pre-existing issues the removal
makes user-visible.

1. packet_show_sock() still bails out to a fallback that no longer exists
---------------------------------------------------------------------

misc/ss.c:4776

	/* use /proc/net/packet if all info are not available */
	if (!tb[PACKET_DIAG_MEMINFO])
		return -1;

This is the last remaining reference to /proc/net/packet in the tree, and
it is not just a stale comment.  That `return -1` was the mechanism for
requesting the fallback: it propagated up so packet_show() would retry
via net_packet_open().  With the fallback gone, the -1 is returned from
an rtnl_dump_filter() callback, which aborts the entire dump.

So on any kernel that answers the sock_diag request but omits
PACKET_DIAG_MEMINFO for a socket, the old code printed that socket from
/proc.  The new code silently truncates the listing at that socket:
everything already dumped before it has been printed, that socket and
every socket after it are dropped, and the exit status is still 0.  If
the very first socket is the one missing MEMINFO, the output is empty.
Before, missing MEMINFO degraded to a less detailed listing; now it
degrades to silently incomplete output with no error.

Note MEMINFO is only requested because packet_show_netlink() sets
PACKET_SHOW_MEMINFO unconditionally (misc/ss.c:4909) -- it is not
something the user asked for, so aborting the dump over it is the wrong
trade even ignoring the dead-fallback issue.

Suggested fix -- drop the check with the parser, and let the existing
`if (tb[PACKET_DIAG_MEMINFO])` guard below at misc/ss.c:4786 do its job
(it is already written to tolerate the attribute being absent):

	-	/* use /proc/net/packet if all info are not available */
	-	if (!tb[PACKET_DIAG_MEMINFO])
	-		return -1;
	-
		stat.type   = r->pdiag_type;

stat.rq simply stays 0 in that case, which is the same thing the netlink
path already does for every other optional attribute.

2. No diagnostic when sock_diag is unavailable
----------------------------------------------

packet_show() and netlink_show() now return the netlink result directly,
and main() (misc/ss.c:6183-6186) ignores the return value:

	if (current_filter.dbs & (1<<NETLINK_DB))
		netlink_show(&current_filter);
	if (current_filter.dbs & PACKET_DBM)
		packet_show(&current_filter);

On a kernel without CONFIG_PACKET_DIAG / CONFIG_NETLINK_DIAG (these are
separate config symbols and are genuinely off in some minimal builds,
independent of kernel age), `ss -0` and `ss -A netlink` now print a
header and nothing else, exit 0, with no indication that anything
failed.  The commit message argues such kernels do not matter, which is
defensible for the fallback itself -- but silently succeeding is not the
same as not supporting it.

This is consistent with how vsock/tipc/xdp already behave, so I would not
block on it.  Still, a one-line stderr hint when the dump fails would
save users a lot of confusion, and it is cheap to add.

3. packet_raw / packet_dgram filtering is now dead in all cases
---------------------------------------------------------------

To be clear up front: this patch did not introduce this bug.  But it
removes the last environment in which the filter ever worked, so it is
worth calling out in the commit message at minimum.

The removed packet_show_line() carried the only type filtering that ever
existed:

> -	if (type == SOCK_RAW && !(f->dbs & (1<<PACKET_R_DB)))
> -		return 0;
> -	if (type == SOCK_DGRAM && !(f->dbs & (1<<PACKET_DG_DB)))
> -		return 0;

There is no equivalent in the netlink path.  packet_show_sock() reads the
type at misc/ss.c:4780:

	stat.type   = r->pdiag_type;

but neither it nor packet_stats_print() ever tests f->dbs, and
run_ssfilter() does not look at s->type.  Since the dispatcher gates on
PACKET_DBM (misc/ss.c:6185), which is the OR of both bits, selecting
either `packet_raw` or `packet_dgram` runs an unfiltered dump and
displays all AF_PACKET sockets.

Because packet_show() already preferred the netlink path before this
patch, `ss --packet_raw` was already returning dgram sockets too on any
kernel with packet_diag.  After this patch it is unconditional.  The
option is still documented (man/man8/ss.8:411) and still accepted by
filter_db_parse() (misc/ss.c:403-407), so users get a silently wrong
answer rather than an error.

The fix is a packet_type_skip() mirroring the existing unix_type_skip()
(misc/ss.c:4443) and vsock_type_skip() (misc/ss.c:5237), called from
packet_show_sock() once stat.type is populated.  That is arguably a
separate patch -- but please either include it here or note the known
gap in the commit message, so it is not mistaken for collateral damage
from this removal.

Minor notes
-----------

- unix_show() keeps its /proc fallback (misc/ss.c:4597).  If the argument
  is "kernels new enough to matter answer sock_diag", it applies equally
  to AF_UNIX.  Worth a sentence in the commit message on why packet and
  netlink were chosen and unix left alone, so the asymmetry reads as
  deliberate.

- The PROC_NET_PACKET / PROC_NET_NETLINK env overrides went away with the
  macros.  I grepped the tree: nothing in man/, README*, or testsuite/
  references them, so there is no doc or test fallout.  Confirming for
  the record, not asking for a change.

- netlink_show_one()'s getenv("PROC_ROOT") at misc/ss.c:5127 is unrelated
  (it resolves a peer PID's name from /proc/<pid>/stat) and correctly
  left alone.

- Builds clean -- no unused-function warnings.  packet_stats_print(),
  netlink_show_one() and generic_record_read() all still have callers on
  the netlink and tcp/udp/raw paths.

I will send its suggested changes