Re: [PATCH iproute2] ss: fix vsock port filter

[Luigi Leonardi <leonardi@redhat.com>]

On Wed, Apr 22, 2026 at 09:21:00AM -0700, Stephen Hemminger wrote:
>On Wed, 22 Apr 2026 10:03:49 +0200
>Stefano Garzarella <sgarzare@redhat.com> wrote:
>
>> On Wed, 22 Apr 2026 at 01:38, Stephen Hemminger <stephen@networkplumber.org> wrote:
>> >
>> > On Tue, 21 Apr 2026 14:35:12 +0200
>> > Luigi Leonardi <leonardi@redhat.com> wrote:
>> >
>> > > parse_hostcond() uses get_u32() to parse the vsock port into the
>> > > aafilter.port field, which is a long. On 64-bit systems, get_u32()
>> > > only writes the lower 32 bits, leaving the upper 32 bits set from
>> > > the -1 initialization. This causes the port comparison
>> > > "a->port != s->rport" in run_ssfilter() to always fail, since the
>> > > corrupted long value never matches the int rport.
>> > >
>> > > Fix by using get_long() instead, consistent with how AF_PACKET and
>> > > AF_NETLINK handle the same field.
>> > >
>> > > Fixes: c759116a0b2b ("ss: add AF_VSOCK support")
>> > > Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>> > > ---
>> > >  misc/ss.c | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/misc/ss.c b/misc/ss.c
>> > > index 14e9f27a..6e3321ac 100644
>> > > --- a/misc/ss.c
>> > > +++ b/misc/ss.c
>> > > @@ -2323,7 +2323,7 @@ void *parse_hostcond(char *addr, bool is_port)
>> > >               port = find_port(addr, is_port);
>> > >
>> > >               if (port && strcmp(port, "*") &&
>> > > -                 get_u32((__u32 *)&a.port, port, 0))
>> > > +                 get_long(&a.port, port, 0))
>> > >                       return NULL;
>> >
>> > If you use get_long() then the code could get negative values.
>> > Actually have port in ss as signed value seems like a mistake in original design.
>> >
>> > The port in unix domain socket is inode number.
>> > Originally it was int, but got changed to long back in 6.6
>> >
>> > The port in ss cache is int.
>>
>> Yeah, as I mentioned I think the issue was introduced by commit
>> 012cb515 ("ss: change aafilter port from int to long (inode support)").
>
>What about this which avoids the cast but keeps the same semantics.
>
>diff --git a/misc/ss.c b/misc/ss.c
>index 14e9f27a..e830e146 100644
>--- a/misc/ss.c
>+++ b/misc/ss.c
>@@ -2317,14 +2317,16 @@ void *parse_hostcond(char *addr, bool is_port)
>
> 	if (fam == AF_VSOCK) {
> 		__u32 cid = ~(__u32)0;
>+		__u32 vport = 0;
>
> 		a.addr.family = AF_VSOCK;
>
> 		port = find_port(addr, is_port);
>-
>-		if (port && strcmp(port, "*") &&
>-		    get_u32((__u32 *)&a.port, port, 0))
>-			return NULL;
>+		if (port && strcmp(port, "*")) {
>+			if (get_u32(&vport, port, 0))
>+				return NULL;
>+		}
>+		a.port = vport;
>
> 		if (!is_port && addr[0] && strcmp(addr, "*")) {
> 			a.addr.bitlen = 32;
>
>

With high enough ports it's not working, I expect to be a problem with
the sign. I'll try updating `struct sockstat` as Stefano suggested.

Luigi