netdev - Re: [PATCH 0/4] net_sched: actions - Add default lookup and walker

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 for Android: free password hash cracker in your pocket

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

Message-ID: <529DDDD4.6070706@mojatatu.com>
Date:	Tue, 03 Dec 2013 08:34:12 -0500
From:	Jamal Hadi Salim <jhs@...atatu.com>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org, eric.dumazet@...il.com,
	alexander.h.duyck@...el.com, ebiederm@...ssion.com
Subject: Re: [PATCH 0/4] net_sched: actions - Add default lookup and walker

On 12/02/13 16:27, David Miller wrote:
>
> Ok, but can you respin this set, adding in changes to remove
> the code that checks for NULL in these operations?
>
> Specifically, we still have:
>
> 	if (a->ops->lookup == NULL)
> 		goto err_mod;
>
> in tcf_action_get_1().  And:
>
> 	if (a_o->walk == NULL) {
>
> in tc_dump_action().
>

Ok, I think i finaly understood what you were saying earlier;->
I was too focused on the simple fix needed for Eric B.
Will make this change ..

> Furthermore, there is a hard assumption that a_o->init() is non-NULL
> as well.  So maybe add a:
>
> 	if (!act->init)
> 		return -EINVAL;
>
> at the top of tcf_register_action().
>

Makes sense.

> It also occurred to me that it would be simpler to audit and be a
> simpler patch set to review if you just added the missing ".lookup ="
> assignments to the actions

Those two methods have proven to be always the defaults and only the
oddball action deviates. So my preference is to have defaults as per
that earlier patch (and overrides for the oddball).


 >and added a similar "if (!act->lookup)" et
> al. check to tcf_register_action().
>
> That looks more like a rigorous requirement to have a valid method
> present for these three operations: init, lookup, walk.
>
> What do you think?

Yep - makes sense (I think you meant act, dump, cleanup and init)
I will add an extra patch..

cheers,
jamal


> Thanks!
>

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