svn commit: r296025 - head/sys/netpfil/pf

[Don Lewis]

On 25 Feb, Konstantin Belousov wrote:
> On Thu, Feb 25, 2016 at 10:17:41AM +0100, Kristof Provost wrote:
>> On 2016-02-24 23:47:55 (-0800), Conrad Meyer <cem at FreeBSD.org> wrote:
>> > On Wed, Feb 24, 2016 at 11:41 PM, Adrian Chadd <adrian.chadd at gmail.com> wrote:
>> > > .. what's capping totlen so one doesn't run out of memory?
>> > 
>> > There was a DoS vector before (user controlled io->pfrio_size) and
>> > basically the same DoS vector now (either of io->pfrio_size or
>> > io->pfrio_size2).  This change isn't a regression.  Still, it should
>> > be fixed.
>> > 
>> It's an M_WAITOK allocation, so if the user asks for more memory than is
>> available the thread will sleep. I'd assumed that if the user terminates
>> the thread the sleep will wake, the allocation will fail and the ioctl()
>> will return an error.
> M_WAITOK allocations still panic when requested amount of KVA is
> unreasonable.
> 
> I am curious what do you mean by 'user terminating the thread'. The
> sleep in malloc() is uninterruptible by signal, and user does not have
> any other way to disturb the execution.
> 
>> 
>> Perhaps we should do what OpenBSD do, and not allocate the temporary
>> buffer at all. They copy in/out the individual entries one by one. On
>> the other hand, one could still exhaust memory by inserting large
>> numbers of addresses in the table.
> But note that accesses to the user memory may fault, which puts whole
> VM and VFS subsystems (and possibly the network as well, if user
> supplied address is backed by mapped NFS file) after the locks owned
> at the moment of copyin() call.

Well, you could wire the user buffer like is done for sysctl buffers.
That removes the KVA constraint, but it basically just moves the problem
elsewhere.  As I recall, we have limits on wired memory, though I
believe it is less restrictive for transient requests.  It's been ages
since I looked at this.