On Sat, Nov 11, 2017 at 11:18 PM, Johannes Berg <johan...@sipsolutions.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.
