On Sat, 15 Dec 2018 at 09:58, David Miller <da...@davemloft.net> wrote:
>
> From: Taehee Yoo <ap420...@gmail.com>
> Date: Wed, 12 Dec 2018 10:19:26 +0900
>
> > When bpfilter error occurred bpfilter_umh will be stopped via __stop_umh().
> > The bpfilter_umh() couldn't start again because there is no restart
> > routine.
> >
> > The section of bpfilter_umh_{start/end} is no longer .init.rodata
> > because these area should be reused in the restart routine. hence
> > the section name is changed to .bpfilter_umh.
> >
> > Test commands:
> > $ iptables -vnL
> > $ kill -9 <pid of bpfilter_umh>
> > $ iptables -vnL
> > [ 480.045136] bpfilter: write fail -32
> >
> > $ iptables -vnL
> > iptables v1.8.1 (legacy): can't initialize iptables table `filter': No
> > child processes
> > Perhaps iptables or your kernel needs to be upgraded.
> >
> > Then, iptables command is always failed.
> >
> > Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
> > Signed-off-by: Taehee Yoo <ap420...@gmail.com>
>
> Thank you for this fix, but I am unsure if this is a complete solution.
>
Thank you for your review!
> First of all, you can only kill the bpfilter_umh as root right?
>
Yes, only root could kill bpfilter_umh process.
> It is a big problem to allow the userspace to kill the umh program
> because that will result in the pipes being leaked. Normally the
> kernel takes down bpfilter_umh and calls fput() on the pipe file
> descriptors via shutdown_umh().
>
> In the kill -9 example above, that does not happen and that is why
> we get the fd leak.
>
> In this situation it will not reset info->pid to zero, and it also
> will leave bpfilter_process_socktop non-NULL.
>
> Therefore, it seems like the kernel has to perform these cleanup
> actions if the bpfilter_umh process dies (from kill -9, or just
> crashing). And I think if you manage that properly it will fix this
> bug too.
If bpfilter_umh process is killed, shutdown_umh() is executed via __stop_umh().
because, __kernel_write() or kernel_read() will be failed in
__bpfilter_process_sockopt() if bpfilter_umh process had killed
or crashed. then, __bpfilter_process_sockopt() makes error message and
calls __stop_umh().
So the cleanup code is called when next iptables command is executed.
$modprobe bpfilter
$kill < pid of bpfilter_umh >
$iptables -vnL <-- stop_umh() is called and iptables command
fails at this point.
[ 512.543626] bpfilter: write fail -32
$iptables -vnL <-- re-start routine will be called and iptables
command will success.
By any chance, should the cleanup action immediately be called
when the bpfilter_umh process is killed?
If you think that I didn't understand correctly what you said,
please let me know.
