netdev - Re: [PATCH net] netlink: fix netlink

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

Message-Id: <20131108.150741.966018155704146843.davem@davemloft.net>
Date:	Fri, 08 Nov 2013 15:07:41 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	jbenc@...hat.com
Cc:	netdev@...r.kernel.org, pablo@...filter.org
Subject: Re: [PATCH net] netlink: fix netlink_ack with large messages

From: Jiri Benc <jbenc@...hat.com>
Date: Thu,  7 Nov 2013 19:57:45 +0100

> Commit c05cdb1b864f ("netlink: allow large data transfers from user-space")
> does not handle cases where netlink_ack is used to report an error. In such
> case, the original message is copied to the ack message, which needs to be
> large enough.
> 
> Signed-off-by: Jiri Benc <jbenc@...hat.com>

I have two problems with this change.

First of all, if netlink_ack() has this problem, do not extend the
netlink_alloc_large_skb() usage to dumps too as your patch is
doing here.

Secondly, it seems sort of over the top to quote such enormous
messages, and in fact wasteful.  We have the sequence number in
the netlink header, so the user can tell exactly which message
we are erroring.

Just quoting such huge requests in ACKs by default doesn't seem
to make any sense.  I would say that we should have a way to
turn off the quoting, or at least limit it, and turn this knob
off for things like nftables that can hit these kinds of cases.

Pablo, what do you think?
--
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