netdev - Re: [PATCH net-next 1/5] net: l2tp: fix L2TP_ATTR_UDP

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

Message-ID: <1478545589.6033.5@x201s.roaming.asbjorn.biz>
Date:   Mon, 07 Nov 2016 21:00:29 +0000
From:   Asbjørn Sloth Tønnesen <asbjorn@...jorn.st>
To:     David Miller <davem@...emloft.net>
Cc:     jchapman@...alix.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, shankerwangmiao@...il.com
Subject: Re: [PATCH net-next 1/5] net: l2tp: fix L2TP_ATTR_UDP_CSUM
 attribute type

Hi David,

Thanks for the review.

On Mon, 07 Nov 2016 13:08:45 -0500 (EST), David Miller <davem@...emloft.net> wrote:
> From: Asbjoern Sloth Toennesen <asbjorn@...jorn.st>
> Date: Fri,  4 Nov 2016 22:48:34 +0000
> 
> > L2TP_ATTR_UDP_CSUM is a flag, and gets read with
> > nla_get_flag, but it is defined as NLA_U8 in
> > the nla_policy.
> > 
> > It appears that this is only publicly used in
> > iproute2, where it's broken, because it's used as
> > a NLA_FLAG, and fails validation as a NLA_U8.
> > 
> > The only place it's used as a NLA_U8 is in
> > l2tp_nl_tunnel_send(), but iproute2 again reads that
> > as a flag, it's therefore always set. Fortunately
> > it is never used for anything, just read.
> > 
> > CC: Miao Wang <shankerwangmiao@...il.com>
> > Signed-off-by: Asbjoern Sloth Toennesen <asbjorn@...jorn.st>
> 
> This is definitely the wrong way to go about this.
> 
> The kernel is everywhere and updating iproute2 is infinitely
> easier for users to do than updating the kernel.
> 
> And in any event, once exported we really should never change
> the API of anything shown to userspace like this.  Just because
> you can't find a user out there doesn't mean it doesn't exist.

Sure, I have submitted a v2 of the patchset, that keeps the
current netlink API intact.

Was unsure how frozen the API was in these outlying corners,
also only tried changing the cases where the kernel side is inconsistently
implemented, ie. kept L2TP_ATTR_{SEND,RECV}_SEQ as u8-flags since it was
used consitently.


> Please instead fix iproute2 to use u8 attributes for this.

Will do (set with u8-flag, read as u8).

-- 
Best regards
Asbjørn Sloth Tønnesen