netdev - Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CALnjE+rUOz2tafyR0QzvwL3R1gxrpe71cM1Z4DedD=ak4RcKoA@mail.gmail.com>
Date:	Thu, 22 Aug 2013 11:04:38 -0700
From:	Pravin Shelar <pshelar@...ira.com>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	netdev <netdev@...r.kernel.org>, Jesse Gross <jesse@...ira.com>
Subject: Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.

On Thu, Aug 22, 2013 at 10:49 AM, Johannes Berg
<johannes@...solutions.net> wrote:
> On Thu, 2013-08-22 at 10:44 -0700, Pravin Shelar wrote:
>
>> > This may be more correct than my patch, but I'm not sure it's worth
>> > spending the memory. Is there going to be any generic netlink family
>> > that actually puts operations into a different module than the family
>> > itself? I doubt that.
>> >
>> I tried to do same, but I do not have access to ops in
>> genl_lock_done() function. therefore I decided to store direct pointer
>> to module in ops struct.
>
> You mean the family? I still don't like this, it'll be a massive
> increase in memory for something like nl80211 that has a lot of
> operations. In fact I think we should get rid of being allowed to
> register single ops and just force a single array to remove the linked
> list as well.
>
Sorry, I meant I do not have access to genl-family in genl_lock_done().
I am using c.data to store ops pointer. Thats why module pointer was
store in ops.

But considering we do not need to hold ref on genl, I can move module
pointer to family.

>> >> +                     c.module = THIS_MODULE;
>> >
>> > THIS_MODULE here is useless, this code is always built-in.
>> >
>> >> +                     if (!try_module_get(ops->module))
>> >> +                             return -EPROTONOSUPPORT;
>> >
>> > Why open-code it? You can just point c.module to the ops module here as
>> > well (because generic netlink is built-in) and save yourself the
>> > try_module_get stuff.
>> >
>>
>> In locked genl case genl-dump operation has call graph as follows:
>> netlink_dump() -> genl_lock_dumpit() -> ops->dump()
>> Therefore I need to take ref on genl module and ops->module.
>
> There's no "genl module", generic netlink is always built in. That was
> my point, you don't need to hold any reference to the "genl module"
> since it doesn't exist, so you can just point to the ops (family)
> module.
>
ok, this make sense.
I will send updated patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html