netdev - Re: [PATCH v3 1/5] netlink: extended ACK reporting

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170411065908.GB1492@salvia>
Date:   Tue, 11 Apr 2017 08:59:08 +0200
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     David Ahern <dsa@...ulusnetworks.com>
Cc:     Johannes Berg <johannes@...solutions.net>,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Benc <jbenc@...hat.com>, jiri@...nulli.us
Subject: Re: [PATCH v3 1/5] netlink: extended ACK reporting

On Mon, Apr 10, 2017 at 09:35:27AM -0600, David Ahern wrote:
> On 4/10/17 9:30 AM, Johannes Berg wrote:
> > On Mon, 2017-04-10 at 09:26 -0600, David Ahern wrote:
> >> On 4/8/17 2:24 PM, Johannes Berg wrote:
> >>> @@ -2300,14 +2332,35 @@ void netlink_ack(struct sk_buff *in_skb,
> >>> struct nlmsghdr *nlh, int err)
> >>>  			  NLMSG_ERROR, payload, 0);
> >>>  	errmsg = nlmsg_data(rep);
> >>>  	errmsg->error = err;
> >>> -	memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg) ? nlh-
> >>>> nlmsg_len : sizeof(*nlh));
> >>> +	memcpy(&errmsg->msg, nlh,
> >>> +	       !(nlk->flags & NETLINK_F_CAP_ACK) ? nlh->nlmsg_len
> >>> +						 : sizeof(*nlh));
> >>> +
> >>
> >> generically this makes userspace parsing more problematic: the
> >> parsing layer may not know if the socket option has been set to
> >> precisely know the size of errmsg->msg and how much data needs to be
> >> skipped to get to the new attributes.
> > 
> > Yes, I know. I'd hope that userspace can remember that per socket - I
> > don't see a good other way to do this.
> > 
> > If we insert the TLVs in front of, or instead of (with a TLV containing
> > it), the request message then at least libnl's debugging will need to
> > be changed.
> > 
> > As it is, I can assume that libnl will not set the CAP setting, and
> > everything works fine even if I don't change libnl, which makes things
> > easier.
> > 
> > Do you have any better ideas?
> 
> NETLINK_F_CAP_ACK and NETLINK_F_EXT_ACK should be incompatible -- if one
> is set the other can not be set. CAP_ACK means abbreviate the response
> and EXT_ACK means give me more data.

CAP_ACK means: trim off the payload that the netlink error message
is embedding, just like ICMP error does.

What is exactly your concern? If the user explicitly requests this via
socket option for this socket, then we're expecting they do the right
handling for what they're asking for.