netdev - Re: [PATCH] netlink: trim skb to exact size to avoid MSG

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: <20151014074436.GE3266@pox.localdomain>
Date:	Wed, 14 Oct 2015 09:44:36 +0200
From:	Thomas Graf <tgraf@...g.ch>
To:	"Arad, Ronen" <ronen.arad@...el.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] netlink: trim skb to exact size to avoid MSG_TRUNC

On 10/13/15 at 05:52pm, Arad, Ronen wrote:
> [@Ronen] My reader as I described above is providing a larger message
> which I'm trying to properly size. I'm aware that libnl shields
> applications from the need to know and provide properly sized buffer by
> peeking or/and re-allocating a buffer.
> My issue is with iproute2 "ip link show" and "bridge vlan show" commands.

>  
> >I'm just trying to understand which exact case you are solving here.
> Allocation is always performed by alloc_size which could be
> nlk->max_recvmsg_len (only when min_dump_alloc is sufficiently small) and
> upon failure falling back to alloc_min_size.
> The trimming of the skb space is common regardless of the allocation call.
> I tried to submit the minimal patch to address the issue. If you think the
> Re-organized code is better I can re-submit a V2.

I was about to suggest the same code change after initial discussion ;-)

So you are fixing the case where >2x messages fit the padded skb size.
This was not clear from the commit message. I would appreciate a note
in the commit message and updated code comment to reflect this.

The fix is definitely not incorrect and the penalty for readers which
peek first is less than I thought since nlk->max_recvmsg_len is at
least 16K in size. Since most peekers will double buffer sizes they
will most likely end up growing nlk->max_recvmsg_len after the first
read.

However, if alloc_size is > 16K, we would have typically ended up with a
giant skb which peeking users were able to take advantage of while
with this fix this is no longer the case.

However #2, I'll see if it makes sense to look at MSG_PEEK in recvmsg
and change nlk->max_recvmsg_len accordingly so we take advantage of
the full skb size on sockets which perform peeking. Given that both
reader behaviours can be preserved, I'm good with your proposed v2.
--
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