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>
index | next in thread | previous in thread | raw e-mail
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.home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?59CA1B6B.5070907>