>>>>> static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
>>>>> {
>>>>> struct cgroup_netprio_state *cs;
>>>>> - int ret;
>>>>> + int ret = -EINVAL;
>>>>>
>>>>> cs = kzalloc(sizeof(*cs), GFP_KERNEL);
>>>>> if (!cs)
>>>>> return ERR_PTR(-ENOMEM);
>>>>>
>>>>> - if (cgrp->parent && cgrp_netprio_state(cgrp->parent)->prioidx) {
>>>>> - kfree(cs);
>>>>> - return ERR_PTR(-EINVAL);
>>>>> - }
>>>>> + if (cgrp->parent && cgrp_netprio_state(cgrp->parent)->prioidx)
>>>>> + goto out;
>>>>>
>>>>> ret = get_prioidx(&cs->prioidx);
>>>>> - if (ret != 0) {
>>>>> + if (ret < 0) {
>>>>> pr_warn("No space in priority index array\n");
>>>>> - kfree(cs);
>>>>> - return ERR_PTR(ret);
>>>>> + goto out;
>>>>> + }
>>>>> +
>>>>> + ret = update_netdev_tables();
>>>>> + if (ret < 0) {
>>>>> + put_prioidx(cs->prioidx);
>>>>> + goto out;
>>>>> }
>>>>
>>>> Gao,
>>>>
>>>> This introduces a null ptr dereference when netprio_cgroup is built
>>>> into the kernel because update_netdev_tables() depends on init_net.
>>>> However cgrp_create is being called by cgroup_init before
>>>> do_initcalls() is called and before net_dev_init().
>>>>
>>>> .John
>>>>
>>> Not sure I follow here John. Shouldn't init_net be initialized prior to any
>>> network devices getting registered? In other words, shouldn't
>>> for_each_netdev
>>> just result in zero iterations through the loop?
>>> Neil
>>>
>>
>> init_net _is_ initialized prior to any network devices getting
>> registered but not before cgrp_create called via cgroup_init.
>>
>> #define for_each_netdev(net, d) \
>> list_for_each_entry(d, &(net)->dev_base_head, dev_list)
>>
>> but dev_base_head is zeroed at this time. In netdev_init we have,
>>
>> INIT_LIST_HEAD(&net->dev_base_head);
>>
>> but we haven't got that far yet because cgroup_init is called
>> before do_initcalls().
>>
> ok, I see that, and it makes sense, but at this point I'm more concerned with
> cgroups getting initalized twice. The early_init flag is clear in the
> cgroup_subsystem for netprio, so we really shouldn't be getting initalized
> from
> cgroup_init. We should be getting initalized from the module_init() call that
> we register
If the early_init flag is set, a cgroup subsys will be initialized from
cgroup_early_init(), otherwise cgroup_init().
If netprio is built as a module, the subsys will be initailized from
module_init(),
otherwise cgroup_init() (in this case cgroup_load_subsys() called in
module_init()
is a no-op).
So it won't get initialized twice.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
