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

[Konstantin Belousov]

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.