public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork•no>
To: Johannes Berg <johannes@sipsolutions•net>
Cc: netdev@vger•kernel.org, Jeff Layton <jeff.layton@primarydata•com>,
	Sedat Dilek <sedat.dilek@gmail•com>
Subject: Re: [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups
Date: Wed, 04 Feb 2015 16:55:42 +0100	[thread overview]
Message-ID: <874mr1isch.fsf@nemi.mork.no> (raw)
In-Reply-To: <1423064608.4741.6.camel@sipsolutions.net> (Johannes Berg's message of "Wed, 04 Feb 2015 16:43:28 +0100")

Johannes Berg <johannes@sipsolutions•net> writes:

> On Wed, 2015-02-04 at 16:36 +0100, Bjørn Mork wrote:
>
>> >> -	int i, err = 0;
>> >> +	int i, err = -ENOENT;
>> >>  
>> >>  	down_read(&cb_lock);
>> >>  	for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
>> >
>> > This change cause serious problems for acpid, as reported on
>> > https://bugzilla.kernel.org/show_bug.cgi?id=92121
>> 
>> Ah, I see this bug is already fixed by commit 8b7c36d810c6 ("netlink:
>> fix wrong subscription bitmask to group mapping in").  Your change was
>> obviously correct, and found the long standing off by one bug.  Thanks.
>> 
>> Sorry about the noise.  I should have checked the current "net" first.
>
> Interesting. I was completely willing to entertain the notion that some
> userspace might be broken and be attempting to subscribe to a (static
> through the hacks we had to put in or "I think I know it already")
> group.

Me to.  So I went through the whole loop, checking that acpid did
everything by the book, adding a debug message to genl_bind() only to
see that it was called with '1' instead of the expected '2'.  Then
looking at af_netlink.c and its history. Etc.

> Have you checked acpid with the bitmap fix?

No, not yet. But I went down far enough into this that I actually wrote
the exact same patch as Pablo. Had to scratch my head when I couldn't
cherry-pick it into net because it was already there :-)

So I am pretty sure Pablo's patch fixes the problem.

This was never noticed before because there are only two users,
netfilter and generic netlink,  and both were willing to accept the
off-by-one values without much fuzz.


Bjørn

  reply	other threads:[~2015-02-04 15:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-16 10:37 [PATCH v3 1/3] genetlink: document parallel_ops Johannes Berg
2015-01-16 10:37 ` [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups Johannes Berg
2015-01-16 22:20   ` David Miller
2015-02-04 11:55   ` Bjørn Mork
2015-02-04 15:36     ` Bjørn Mork
2015-02-04 15:43       ` Johannes Berg
2015-02-04 15:55         ` Bjørn Mork [this message]
2015-02-04 16:15           ` Bjørn Mork
2015-02-04 16:16           ` Johannes Berg
2015-01-16 10:37 ` [PATCH v3 3/3] genetlink: synchronize socket closing and family removal Johannes Berg
2015-01-16 22:20   ` David Miller
2015-01-16 22:19 ` [PATCH v3 1/3] genetlink: document parallel_ops David Miller

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=874mr1isch.fsf@nemi.mork.no \
    --to=bjorn@mork$(echo .)no \
    --cc=jeff.layton@primarydata$(echo .)com \
    --cc=johannes@sipsolutions$(echo .)net \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=sedat.dilek@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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