From: Al Viro <viro@ftp.linux.org.uk>
To: Stephen Hemminger <shemminger@osdl.org>
Cc: Andrew Morton <akpm@osdl.org>,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH] handle module ref count on sysctl tables.
Date: Wed, 21 Dec 2005 19:08:49 +0000 [thread overview]
Message-ID: <20051221190849.GM27946@ftp.linux.org.uk> (raw)
In-Reply-To: <20051221103520.258ced08@dxpl.pdx.osdl.net>
On Wed, Dec 21, 2005 at 10:35:19AM -0800, Stephen Hemminger wrote:
> Right now there is a hole in the module ref counting system because
> there is no proper ref counting for sysctl tables used by modules.
> This means that if an application is holding /proc/sys/foo open and
> module that created it is unloaded, then the application touches the
> file the kernel will oops.
>
> This patch fixes that by maintaining source compatibility via macro.
> I am sure someone already thought of this, it just doesn't appear to
> have made it in yet.
NAK.
a) holding the file open will *NOT* pin any module structures down.
IO in progress will, but it unregistering sysctl table will block until it's
over. The same goes for sysctl(2) in progress. See use_table() and
friends in kernel/sysctl.c
b) you are not protecting any code in module; what needs protection
(and gets it) is a pile of data structures. With lifetimes that don't have
to be related to module lifetimes. IOW, use of reference to module is 100%
wrong here - it wouldn't fix anything.
As a general rule, when you pin something down, think what you are trying
to protect; if it's not just a bunch of function references - module is
the wrong thing to hold.
In particular, sysctl tables are dynamically created and removed in a
kernel that is not modular at all. Which kills any hope to get a solution
based on preventing rmmod.
Solution is fairly simple:
* put use counter into sysctl table head (i.e. object allocated by
kernel/sysctl.c)
* bump use counter when examining table in sysctl(2) and around the
actual IO in procfs access; put reference to table into proc_dir_entry to
be able to do the latter. Decrement when done with the table; if it had
hit zero _and_ there's unregistration waiting for completion - kick it.
* have unregistration kill all reference to table head and if use
counter is positive - wait for completion. Once we get it, we know that
we can safely proceed.
next prev parent reply other threads:[~2005-12-21 19:08 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-12-21 18:35 [PATCH] handle module ref count on sysctl tables Stephen Hemminger
2005-12-21 18:42 ` Arjan van de Ven
2005-12-21 18:47 ` Stephen Hemminger
2005-12-21 19:08 ` Al Viro [this message]
2005-12-21 19:20 ` Al Viro
2005-12-21 19:21 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20051221190849.GM27946@ftp.linux.org.uk \
--to=viro@ftp.linux.org.uk \
--cc=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=shemminger@osdl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).