Date: Tue, 26 Sep 2017 16:18:35 +0700 From: Eugene Grosbein <eugen@grosbein.net> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r323873 - head/sys/netgraph Message-ID: <59CA1B6B.5070907@grosbein.net> In-Reply-To: <20170925205929.GW1055@FreeBSD.org> References: <201709212016.v8LKGBMi024412@repo.freebsd.org> <20170925205929.GW1055@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 26.09.2017 03:59, Gleb Smirnoff wrote: > On Thu, Sep 21, 2017 at 08:16:11PM +0000, Eugene Grosbein wrote: > E> Author: eugen (ports committer) > E> Date: Thu Sep 21 20:16:10 2017 > E> New Revision: 323873 > E> URL: https://svnweb.freebsd.org/changeset/base/323873 > E> > E> Log: > E> Unprotected modification of ng_iface(4) private data leads to kernel panic. > E> Fix a race with per-node read-mostly lock and refcounting for a hook. > > The patch is far from ideal. Netgraph already has internal locking, > which guarantess write semantics for "disconnect" and "newhook" > scenarios. As well as read semantics for "rcvdata". > > Since ng_iface is a gate node, that gates data between netgraph and the > big network stack, it of course needs extra locking in the interface > output method. But better piggyback on the netgraph locking, rather than > add your own, IMHO. I'm afraid you have not read the change well enough. It does utilizes generic netgraph locking like NG_HOOK_REF(hook) for entities visible to base netgraph code. And it uses own locking only to protect private ng_iface structures from parallel modifications with another kernel thread runnning same ng_iface code. It uses lightweight rmlock there as that is network hot path and anyway, other netgraph code has nothing to do with node's private data.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?59CA1B6B.5070907>