Re: Question on rhashtable in worst-case scenario.

[Ben Greear]

On 03/31/2016 12:46 AM, Johannes Berg wrote:
> On Wed, 2016-03-30 at 09:52 -0700, Ben Greear wrote:
>
> > If someone can fix rhashtable, then great.
> > I read some earlier comments [1] back when someone else reported
> > similar problems, and the comments seemed to indicate that rhashtable
> > was broken in this manner on purpose to protect against hashing
> > attacks.
> >
> > If you are baking in this type of policy to what should be a basic
> > data-type, then it is not useful for how it is being used in
> > the mac80211 stack.
> >
> > [1]  http://lkml.iu.edu/hypermail/linux/kernel/1512.2/01681.html
>
> That's not really saying it's purposely broken, that's more saying that
> Herbert didn't see a point in fixing a case that has awful behaviour
> already.
>
> However, I'm confused now - we can much more easily live with
> *insertion* failures, as the linked email indicates, than *deletion*
> failures, which I think you had indicated originally. Are you really
> seeing *deletion* failures?
>
> If there are in fact *deletion* failures then I think we really need to
> address those in rhashtable, no matter the worst-case behaviour of the
> hashing or keys, since we should be able to delete entries in order to
> get back to something reasonable. Looking at the code though, I don't
> actually see that happening.
>
> If you're seeing only *insertion* failures, which you indicated in the
> root of this thread, then I think for the general case in mac80211 we
> can live with that - we use a seeded jhash for the hash function, and
> since in the general case we cannot accept entries with identical MAC
> addresses to start with, it shouldn't be possible to run into this
> problem under normal use cases.

I see insertion failure, and then later, if of course fails to delete
as well since it was never inserted to begin with.  There is no good
way to deal with insertion error, so just need to fix the hashtable.

> In this case, I think perhaps you can just patch your local system with
> the many interfaces connecting to the same AP to add the parameter
> Herbert suggested (.insecure_elasticity = true in sta_rht_params). This
> is, after all, very much a case that "normal" operation doesn't even
> get close to.

Old code, even stock kernels, could deal with this properly, so I think it
should be fixed by default.  I'll put rhash back in my tree and try that insecure
option and see if it works.

Thanks,
Ben
> johannes

--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc  http://www.candelatech.com