netdev - Re: [PATCH 2/2] genetlink: convert family dump code to use RCU

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1377155850.14110.15.camel@jlt4.sipsolutions.net>
Date:	Thu, 22 Aug 2013 09:17:30 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Pravin Shelar <pshelar@...ira.com>
Cc:	netdev <netdev@...r.kernel.org>, Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH 2/2] genetlink: convert family dump code to use RCU

On Wed, 2013-08-21 at 21:32 -0700, Pravin Shelar wrote:

> I send out a fix which fixes genl locking for dump operation and it
> might fix this issue, Can you try this?

I don't think that fix is correct. I don't have a copy, so let me reply
here.

> http://marc.info/?l=linux-netdev&m=137714389705485&w=2

> In case of genl-family with parallel ops off, dumpif() callback
> is expected to run under genl_lock, 
> But commit def3117493eafd9df
> (genl: Allow concurrent genl callbacks.) changed this behaviour
> where only first dumpit() op was called under genl-lock.

I don't think either of those statements are true - dump() has alway
taken a shortcut in netlink_recvmsg() and doesn't go into genl_rcv(), so
how could your patch have changed locking? Therefore, it can't have been
expected either.

> For subsequent dump, only nlk->cb_lock was taken.
> Following patch fixes it by defining locked dumpit() and done()
> callback which takes care of genl-locking.

This might help for generic netlink itself, but I'm not convinced that
it's really useful for other families. I've fixed bugs like in commit
3a5a423bb958ad22eeccca66c533e85bf69ba10e, which actually made it in
after your commit, but per above I don't think it was actually a problem
introduced by your commit.

johannes

--
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