Re: Race condition in ipv6 code

[ebiederm@xmission.com (Eric W. Biederman)]

Ben Greear <greearb@candelatech.com> writes:

> On 01/12/2012 11:40 PM, Eric W. Biederman wrote:
>
>> So I really think the best solution to avoid the locking craziness is to
>> have a wrapper that gets the value from userspace and calls
>> schedule_work to get another thread to actually process the change.  I
>> don't see any problems with writing a helper function for that.  The
>> only downside with using schedule_work is that we return to userspace
>> before the change has been fully installed in the kernel.  I don't
>> expect that would be a problem but stranger things have happened.
>
> That sounds a bit risky to me.  If something sets a value, and then
> queries it, it should always show the proper result for the previous
> calls.

Which is easy to do if you keep two values.  One integer
for the userspace control and another integer for the internal
kernel state.

The problem is that we have exactly one integer currently.

> If the queries also went through the the same sched-work queue
> then maybe it would be OK.

We can't want for anything that has to take the rtnl_lock.  That would
be the same as taking the rtnl_lock from a locking perspective.

I expect I would use something like:
struct rtnl_protected_knob {
	struct work_struct work;
        int userspace_value;
        int *kernel_var;
        void (*func)(int new_value, *kernel_var);
};

userspace_value would be what userspace sees, and kernel_var would be a
pointer to the value that we manipulate in the kernel.

Eric