netdev - Re: bug in ip -s xfrm state list

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>] [day] [month] [year] [list]

Message-ID: <20120530081338.1904c604@nehalam.linuxnetplumber.net>
Date:	Wed, 30 May 2012 08:13:38 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Jaroslav Šafka <jaroslav.safka@...mens.com>
Cc:	netdev@...r.kernel.org
Subject: Re: bug in ip -s xfrm state list

On Wed, 30 May 2012 15:00:05 +0200
Jaroslav Šafka <jaroslav.safka@...mens.com> wrote:

> Hello guys,
> I found small problem in printing "seq" number.
> I think the problem is visible in diff below ;-)
> 
> Best regards
> Jarek
> 
> diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
> index c7b3420..67fe838 100644
> --- a/ip/ipxfrm.c
> +++ b/ip/ipxfrm.c
> @@ -843,7 +843,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info 
> *xsinfo,
>         fputs(buf, fp);
>         fprintf(fp, "replay-window %u ", xsinfo->replay_window);
>         if (show_stats > 0)
> -               fprintf(fp, "seq 0x%08u ", xsinfo->seq);
> +               fprintf(fp, "seq %08u ", ntohl(xsinfo->seq));
>         if (show_stats > 0 || xsinfo->flags) {
>                 __u8 flags = xsinfo->flags;
>  
> 
> 

Moving to netdev list for more feedback.
Your change has two components:
  1. printing sequence number in decimal notation with 0x prefix.
  2. doing ntohl().

The first is an obvious bug in the original code. Not sure about
the second. It looks like sequence numbers generated by the kernel
come from xfrm_get_acqseq() which generates sequence numbers in host
byte order. But sequence numbers entered from user space (vi xfrm_seq_parse)
are encoded in network byte order. This looks like a flaw in the original design.

The kernel networking code tries to be careful about documenting network
verus host byte order via the typedef __be32 vs __u32.  Given that the kernel
is using __u32 for sequence number in xfrm fairly consistently; my opninon
is that iproute2 should change and go with the kernel practice and
use host byte order.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html