[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 12 Nov 2017 00:18:40 +0900
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Johannes Berg <johannes@...solutions.net>
Cc: David Miller <davem@...emloft.net>,
Netdev <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] af_netlink: ensure that NLMSG_DONE never fails in dumps
On Sat, Nov 11, 2017 at 11:18 PM, Johannes Berg
<johannes@...solutions.net> wrote:
>
>> > If you're handling this by forcing another read() to procude the
>> > NLMSG_DONE, then you have no reason to WARN_ON() here.
>> >
>> > In fact you are adding a WARN_ON() which is trivially triggerable by
>> > any user.
>>
>> I added this in my suggestion for how this could work, but I don't
>> think you're right, since we previously check if there's enough space.
>
> Or perhaps I should say this differently:
>
> Forcing another read happens through the
>
> skb_tailroom(skb) < nlmsg_total_size(...)
>
> check, so the nlmsg_put_answer() can't really fail.
>
>
> Handling nlmsg_put_answer() failures by forcing another read would have
> required jumping to the existing if code with a goto, or restructuring
> the whole thing completely somehow, and I didn't see how to do that.
Exactly. And if something _does_ go wrong in our logic, and we can't
add NLMSG_DONE, we really do want people to report this to us, since
dumps must always end that way. We'd probably have caught this a
number of years ago when userspace developers were twiddling with
their receive buffers if we had had the WARN_ON. Nice suggestion from
Johannes.
Powered by blists - more mailing lists